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

fix (#759): fixed axios versions inconsistency #827

Merged

Conversation

martinsefcik
Copy link
Contributor

@martinsefcik martinsefcik commented Oct 30, 2023

Description

Fixes #759.

Inconsistency of Axios versions defined in Bruno packages. Then axios requests can behave differently in bruno-cli and bruno-app both executed from official binaries or also differently in bruno-app executed from binaries and bruno-app executed from the sources.

Contribution Checklist:

  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

@martinsefcik
Copy link
Contributor Author

@helloanoop any chance to have it merged soon ? It blocks us to use our big production collection with bruno-cli

@helloanoop helloanoop merged commit 1e6c85e into usebruno:main Nov 6, 2023
2 checks passed
@helloanoop
Copy link
Contributor

@martinsefcik This is now released in v1.1.1 🎉

@martinsefcik
Copy link
Contributor Author

martinsefcik commented Nov 9, 2023

@helloanoop thanks but important for me is to have it released in bruno-cli and there is the latest version 1.0.0 I expect without this fix.

@helloanoop
Copy link
Contributor

@martinsefcik I just released bruno-cli v1.1.0 which uses axios 1.5.1

@martinsefcik
Copy link
Contributor Author

martinsefcik commented Nov 9, 2023

@helloanoop I appreciate your effort but it still does not work, probably because published @usebruno/js 0.9.1 npm package does not contain changes I did in this PR in this package. So I think it is needed to release new 0.9.2 version of @usebruno/js then use this 0.9.2 version in bruno-cli and then do new release @usebruno/cli 1.1.1

It can be then easily tested by putting the following code in any request's pre-request script:

console.log(axios.defaults.transformRequest.toString())

if it prints this:

function transformRequest(data, headers) {
    normalizeHeaderName(headers, 'Accept');
    normalizeHeaderName(headers, 'Content-Type');

    if (utils.isFormData(data) ||
      utils.isArrayBuffer(data) ||
      utils.isBuffer(data) ||
      utils.isStream(data) ||
      utils.isFile(data) ||
      utils.isBlob(data)
    ) {
      return data;
    }
    if (utils.isArrayBufferView(data)) {
      return data.buffer;
    }
    if (utils.isURLSearchParams(data)) {
      setContentTypeIfUnset(headers, 'application/x-www-form-urlencoded;charset=utf-8');
      return data.toString();
    }
    if (utils.isObject(data) || (headers && headers['Content-Type'] === 'application/json')) {
      setContentTypeIfUnset(headers, 'application/json');
      return stringifySafely(data);
    }
    return data;
  }

then some pre 1.0.0 version of axios was bundled in bruno-cli (current state with bruno-cli 1.1.1)

If it prints more complex implementation of default request transform function (with support for transforming payload object to url form encoded data):

function transformRequest(data, headers) {
    const contentType = headers.getContentType() || '';
    const hasJSONContentType = contentType.indexOf('application/json') > -1;
    const isObjectPayload = utils.isObject(data);

    if (isObjectPayload && utils.isHTMLForm(data)) {
      data = new FormData(data);
    }

    const isFormData = utils.isFormData(data);

    if (isFormData) {
      if (!hasJSONContentType) {
        return data;
      }
      return hasJSONContentType ? JSON.stringify(formDataToJSON(data)) : data;
    }

    if (utils.isArrayBuffer(data) ||
      utils.isBuffer(data) ||
      utils.isStream(data) ||
      utils.isFile(data) ||
      utils.isBlob(data)
    ) {
      return data;
    }
    if (utils.isArrayBufferView(data)) {
      return data.buffer;
    }
    if (utils.isURLSearchParams(data)) {
      headers.setContentType('application/x-www-form-urlencoded;charset=utf-8', false);
      return data.toString();
    }

    let isFileList;

    if (isObjectPayload) {
      if (contentType.indexOf('application/x-www-form-urlencoded') > -1) {
        return toURLEncodedForm(data, this.formSerializer).toString();
      }

      if ((isFileList = utils.isFileList(data)) || contentType.indexOf('multipart/form-data') > -1) {
        const _FormData = this.env && this.env.FormData;

        return toFormData(
          isFileList ? {'files[]': data} : data,
          _FormData && new _FormData(),
          this.formSerializer
        );
      }
    }

    if (isObjectPayload || hasJSONContentType ) {
      headers.setContentType('application/json', false);
      return stringifySafely(data);
    }

    return data;
  }

then some 1.0.0+ version of axios is bundled in bruno-cli (expected state)

@helloanoop
Copy link
Contributor

Thanks @martinsefcik !

I've just release v1.1.1.

image

@martinsefcik
Copy link
Contributor Author

@helloanoop Thank you very much, it is working fine now!

ParamjotSingh5 added a commit to ParamjotSingh5/bruno that referenced this pull request Nov 16, 2023
* Implimented logic of cyclic traversal of tabs array with % opreator.
helloanoop added a commit that referenced this pull request Aug 29, 2024
* feat(#736): Switch tabs with keyboard shortcut

1. Registered keyboard events in Hotkeys/index.js
2. Added logic for replacing `state.activeTabUid` to switch active tab as per keyboard event.
3. Maintained a stack `recentUsedTabsStack` for tab visit history and pop out on `Ctrl+Tab` keyboard event.

* feat(#736): Switch tabs with keyboard shortcut

Keeping this feature request only limited to CTRL+PGUP and CTRL_PGDN button clicks functionality. Hence removing logic for CTRL+TAB click functionality.

* feat(#736): Switch tabs with keyboard shortcut

clean up

* feate(#827): Switch tabs with keyboard shortcut

* Implimented logic of cyclic traversal of tabs array with % opreator.

---------

Co-authored-by: Anoop M D <[email protected]>
Its-treason pushed a commit to Its-treason/bruno that referenced this pull request Aug 29, 2024
* feat(usebruno#736): Switch tabs with keyboard shortcut

1. Registered keyboard events in Hotkeys/index.js
2. Added logic for replacing `state.activeTabUid` to switch active tab as per keyboard event.
3. Maintained a stack `recentUsedTabsStack` for tab visit history and pop out on `Ctrl+Tab` keyboard event.

* feat(usebruno#736): Switch tabs with keyboard shortcut

Keeping this feature request only limited to CTRL+PGUP and CTRL_PGDN button clicks functionality. Hence removing logic for CTRL+TAB click functionality.

* feat(usebruno#736): Switch tabs with keyboard shortcut

clean up

* feate(usebruno#827): Switch tabs with keyboard shortcut

* Implimented logic of cyclic traversal of tabs array with % opreator.

---------

Co-authored-by: Anoop M D <[email protected]>
berlingoqc pushed a commit to berlingoqc/OpenBruno that referenced this pull request Sep 5, 2024
* feat(usebruno#736): Switch tabs with keyboard shortcut

1. Registered keyboard events in Hotkeys/index.js
2. Added logic for replacing `state.activeTabUid` to switch active tab as per keyboard event.
3. Maintained a stack `recentUsedTabsStack` for tab visit history and pop out on `Ctrl+Tab` keyboard event.

* feat(usebruno#736): Switch tabs with keyboard shortcut

Keeping this feature request only limited to CTRL+PGUP and CTRL_PGDN button clicks functionality. Hence removing logic for CTRL+TAB click functionality.

* feat(usebruno#736): Switch tabs with keyboard shortcut

clean up

* feate(usebruno#827): Switch tabs with keyboard shortcut

* Implimented logic of cyclic traversal of tabs array with % opreator.

---------

Co-authored-by: Anoop M D <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Axios requests in pre-request scripts behave differently in bruno-app and bruno-cli
2 participants