Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Useless param of Compose method in options.dart #1934

Closed
Q00 opened this issue Aug 10, 2023 · 1 comment · Fixed by #1935
Closed

Useless param of Compose method in options.dart #1934

Q00 opened this issue Aug 10, 2023 · 1 comment · Fixed by #1935
Labels
fixed p: dio Targeting `dio` package s: bug Something isn't working

Comments

@Q00
Copy link

Q00 commented Aug 10, 2023

Request Statement

 RequestOptions compose(
    BaseOptions baseOpt,
    String path, {
    Object? data,
    Map<String, dynamic>? queryParameters,
    CancelToken? cancelToken,
    Options? options,
    ProgressCallback? onSendProgress,
    ProgressCallback? onReceiveProgress,
    StackTrace? sourceStackTrace,
  }) {
    final query = <String, dynamic>{};
    query.addAll(baseOpt.queryParameters);
    if (queryParameters != null) query.addAll(queryParameters);

    final headers = caseInsensitiveKeyMap(baseOpt.headers);
    if (this.headers != null) {
      headers.addAll(this.headers!);
    }
    if (this.contentType != null) {
      headers[Headers.contentTypeHeader] = this.contentType;
    }
    final String? contentType = headers[Headers.contentTypeHeader];
    final extra = Map<String, dynamic>.from(baseOpt.extra);
    if (this.extra != null) {
      extra.addAll(this.extra!);
    }
    final method = (this.method ?? baseOpt.method).toUpperCase();
    final requestOptions = RequestOptions(
      method: method,
      headers: headers,
      extra: extra,
      baseUrl: baseOpt.baseUrl,
      path: path,
      data: data,
      sourceStackTrace: sourceStackTrace ?? StackTrace.current,
      connectTimeout: baseOpt.connectTimeout,
      sendTimeout: sendTimeout ?? baseOpt.sendTimeout,
      receiveTimeout: receiveTimeout ?? baseOpt.receiveTimeout,
      responseType: responseType ?? baseOpt.responseType,
      validateStatus: validateStatus ?? baseOpt.validateStatus,
      receiveDataWhenStatusError:
          receiveDataWhenStatusError ?? baseOpt.receiveDataWhenStatusError,
      followRedirects: followRedirects ?? baseOpt.followRedirects,
      maxRedirects: maxRedirects ?? baseOpt.maxRedirects,
      persistentConnection:
          persistentConnection ?? baseOpt.persistentConnection,
      queryParameters: query,
      requestEncoder: requestEncoder ?? baseOpt.requestEncoder,
      responseDecoder: responseDecoder ?? baseOpt.responseDecoder,
      listFormat: listFormat ?? baseOpt.listFormat,
      onReceiveProgress: onReceiveProgress,
      onSendProgress: onSendProgress,
      cancelToken: cancelToken,
      contentType: contentType ?? this.contentType ?? baseOpt.contentType,
    );
    requestOptions.cancelToken?.requestOptions = requestOptions;
    return requestOptions;
  }

the options parameter is not used in this method.

Solution Brainstorm

We have to merge objects.
I.g. options.addAll(baseOpt)

@Q00 Q00 added the s: feature This issue indicates a feature request label Aug 10, 2023
@AlexV525 AlexV525 added the p: dio Targeting `dio` package label Aug 11, 2023
@AlexV525
Copy link
Member

It might be a bug, I'll run git blame to see if it's intended before we brainstorm this.

@AlexV525 AlexV525 added s: bug Something isn't working and removed s: feature This issue indicates a feature request labels Aug 12, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 13, 2023
Removes the accidentally added `options` argument for `Options.compose`
by 75cf916.

Resolves #1934.

### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [ ] I have added the required tests to prove the fix/feature I'm
adding
- [ ] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package

---------

Signed-off-by: Alex Li <github@alexv525.com>
@AlexV525 AlexV525 added the fixed label Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed p: dio Targeting `dio` package s: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants