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

feat: switch buffer to native Uint8Array #2288

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

char0n
Copy link
Member

@char0n char0n commented Oct 13, 2021

This is followup for #2243.

This is technically not a breaking change as we already claim for long time that minimum version of Node.js is 12.4.

Types of changes

  • No code changes (changes to documentation, CI, metadata, etc)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@char0n
Copy link
Member Author

char0n commented Oct 13, 2021

@jimmywarting please have a look at this PR. It tries to eliminate the usage of Buffer entirely.

@char0n
Copy link
Member Author

char0n commented Oct 13, 2021

@mathis-m please have a look at this well if/when you have time please. Thanks!

Copy link

@mathis-m mathis-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for what is the Fallback?

@char0n char0n force-pushed the char0n/replace-buffer-with-native-api branch from d1e11f5 to 210a7cb Compare October 14, 2021 06:14
@char0n
Copy link
Member Author

char0n commented Oct 14, 2021

@mathis-m good point. I've added explicit explanation for it as a JSDOC of fallback

/**
 * This is a webpack@5 fallback for `buffer` library.
 * swagger-js is using `querystring-browser` which depends on
 * `buffer` under the hood. The usage is compatible with
 * `Uint8Array` interface though.
 *
 * If we introduce other libraries in future that will depend on
 * `buffer`, this fallback needs to be reviewed again.
 */

@jimmywarting
Copy link
Contributor

lgtm

@char0n char0n force-pushed the char0n/replace-buffer-with-native-api branch from 210a7cb to 354f4df Compare October 14, 2021 07:32
@char0n
Copy link
Member Author

char0n commented Oct 14, 2021

Thanks guys.

I've just pushed a commit which eliminates the fallback as it will cause problems for library consumers. I've replaced the querystring-browser library with native calls to UrlSearchParams which is suggested by Node.js documentation as querystring API is now deprecated.

@char0n char0n merged commit e96702f into master Oct 14, 2021
swagger-bot pushed a commit that referenced this pull request Oct 14, 2021
# [3.17.0](v3.16.1...v3.17.0) (2021-10-14)

### Features

* switch buffer to native Uint8Array ([#2288](#2288)) ([e96702f](e96702f)), closes [#2243](#2243)
@swagger-bot
Copy link
Contributor

🎉 This PR is included in version 3.17.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants