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(cognito): use npm package over core module #11387

Closed
wants to merge 2 commits into from

Conversation

meMuszr
Copy link

@meMuszr meMuszr commented May 18, 2023

Description of changes

Avoid core modules for browser builds

Issue #, if available

Description of how you validated changes

Checklist

  • [ x] PR description included
  • [ x] yarn test passes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@meMuszr meMuszr requested a review from a team as a code owner May 18, 2023 21:56
@meMuszr
Copy link
Author

meMuszr commented May 22, 2023

@jimblanc something you can have a look at please?

@nadetastic
Copy link
Contributor

Hi @meMuszr thank you for your contribution. Looking at the changes that you are making, they may re-introduce a recent issue that was resolved by this PR - where it essential does the opposite of what your PR is doing. Due to this, I’ll go ahead and close this PR out. Thanks!

@nadetastic nadetastic closed this Jun 22, 2023
@meMuszr
Copy link
Author

meMuszr commented Jun 27, 2023

@nadetastic thanks for your comment. Bummer it got closed prematurely, open to a discussion or no? Looks like the buffer dependency in both that package the linked PR is addressing as well as this one is sitting two major versions behind (and more importantly in the cognito-identity-js package not getting used). Have other solutions been ruled out? Otherwise I'll go ahead and file an issue against the fact that the node module buffer is being used incorrectly in a web context.

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.

None yet

2 participants