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

Webpack 5 compatibility #8579

Closed
3 tasks done
rajinder-yadav opened this issue Jul 13, 2021 · 9 comments
Closed
3 tasks done

Webpack 5 compatibility #8579

rajinder-yadav opened this issue Jul 13, 2021 · 9 comments
Assignees
Labels
Angular Related to Angular 2+ feature-request Request a new feature

Comments

@rajinder-yadav
Copy link

Before opening, please confirm:

JavaScript Framework

Angular

Amplify APIs

Authentication

Amplify Categories

auth

Environment information

# Put output below this line

  System:
    OS: Linux 5.13 openSUSE Tumbleweed 20210710
    CPU: (16) x64 AMD Ryzen 7 2700X Eight-Core Processor
    Memory: 16.52 GB / 31.29 GB
    Container: Yes
    Shell: 5.1.8 - /bin/bash
  Binaries:
    Node: 14.17.3 - ~/.nvm/versions/node/v14.17.3/bin/node
    Yarn: 1.22.10 - ~/.nvm/versions/node/v14.17.3/bin/yarn
    npm: 7.19.1 - ~/.nvm/versions/node/v14.17.3/bin/npm
  Browsers:
    Brave Browser: 91.1.26.74
    Chrome: 91.0.4472.114
    Firefox: 89.0.2
  npmGlobalPackages:
    npm: 7.19.1
    yarn: 1.22.10

Describe the bug

I am making use of Cognito in my Angular 12 app, including the following

import {
	CognitoUserPool,
	CognitoUserAttribute,
	CognitoUser,
} from 'amazon-cognito-identity-js';

I am seeing the following messages in my Angular 12 app:

".../aws-cognito/aws-cognito-register-user/node_modules/amazon-cognito-identity-js/es/AuthenticationHelper.js depends on 'buffer'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies"

My question is why is a Node.js include being used for a frontend browser app, (I believe) this is causing problems with Webpack 5.

snap-20210713_000716

Expected behavior

Angular 12 app should build with any warning message using the Cognito npm module

Reproduction steps

Create a Angular 12 app
install npm module amazon-cognito-identity-js
code a simple registration page
build the app and see tons of build warnings

using the following for reference code: https://github.com/aws-amplify/amplify-js/tree/main/packages/amazon-cognito-identity-js

Code Snippet

// Put your code below this line.

Log output

// Put your logs below this line


aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

@rajinder-yadav rajinder-yadav changed the title AuthenticationHelper.js depends on 'buffer' AuthenticationHelper.js depends on 'buffer' warning with Angular 12 builds Jul 13, 2021
@rajinder-yadav
Copy link
Author

Also seeing warning for crypto-js:

"/node_modules/amazon-cognito-identity-js/es/AuthenticationHelper.js depends on 'crypto-js/core'. CommonJS or AMD dependencies can cause optimization bailouts.
For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies"

@chrisbonifacio chrisbonifacio added the Angular Related to Angular 2+ label Jul 14, 2021
@chrisbonifacio
Copy link
Contributor

When I searched for the buffer dependency mentioned, the npm page says it is for the browser.
https://www.npmjs.com/package/buffer

The page you linked mentions you can disable those warnings

To disable these warnings, you can add the CommonJS module name to allowedCommonJsDependencies option in the build options located in angular.json file.

"build": {
  "builder": "@angular-devkit/build-angular:browser",
  "options": {
     "allowedCommonJsDependencies": [
        "lodash"
     ]
     ...
   }
   ...
},

@chrisbonifacio chrisbonifacio added this to Needs Triage in Issues Triaging via automation Jul 14, 2021
@rajinder-yadav
Copy link
Author

rajinder-yadav commented Jul 14, 2021

Thanks @chrisbonifacio I should have checked package.json about "buffer", the first error message for "crypto-js" threw me off about pollyfills for node.js.

I was able to silence the optimization warnings with respect to CommonJS or AMD modules by adding the following to angular.son (However the solution here would be to use ECMAScript modules so the Angular compiler can tree-shake and reduce the bundle size).

            "allowedCommonJsDependencies": [
              "buffer",
              "crypto-js",
              "isomorphic-unfetch"
            ],

The only warning error now is the breaking change with webpack 5. Not sure what has to be done here, or if Cognito SDK requires the missing polyfill? I feel like this is something that should be cleaned up on the SDK side.

"Module not found: Error: Can't resolve 'crypto' in '/home/yadav/websites/hdt/aws-cognito/aws-cognito-register-user/node_modules/crypto-js'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
	- add a fallback 'resolve.fallback: { "crypto": require.resolve("crypto-browserify") }'
	- install 'crypto-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
	resolve.fallback: { "crypto": false }"

@chrisbonifacio
Copy link
Contributor

chrisbonifacio commented Jul 15, 2021

@rajinder-yadav Yup, that error message is spot on and yes, that would be the ideal solution. I did bring this to the team yesterday and they're currently looking into handling this on our side.

@chrisbonifacio chrisbonifacio moved this from Needs Triage to Feature Requests in Issues Triaging Jul 15, 2021
@chrisbonifacio chrisbonifacio added the feature-request Request a new feature label Jul 15, 2021
@chrisbonifacio chrisbonifacio changed the title AuthenticationHelper.js depends on 'buffer' warning with Angular 12 builds Webpack 5 compatibility Jul 15, 2021
@hkjpotato
Copy link
Contributor

@rajinder-yadav Hi thank you for cutting the issue and you are right, this is a known issue as webpack 5 no longer shim in browserify version of node-js built-in. (In webpack 4, the browserify version of the node-js built in is a direct dependency of webpack)

For crypto-js, it is due to its explicitly require call on the node-js built-in crypto, I have a PR brix/crypto-js#364 in its repo, pending the owner to review.

For buffer, I need to investigate more to see how we can handle it. I am looking at some related issues now:
webpack/changelog-v5#10
diegomura/react-pdf#1029
https://stackoverflow.com/questions/8880571/convert-nodejs-buffer-to-browsers-javascript

@rajinder-yadav
Copy link
Author

Thanks @hkjpotato and @chrisbonifacio for looking into this issue, happy to hear progress is getting made! Looking forward to see Angular 12 building cleanly again :-D

@hkjpotato appreciate the explanation of the issues, it helped with gaining a better understanding of the cause.

@hkjpotato
Copy link
Contributor

@rajinder-yadav Hi, we have updated the crypto-js dependency of Amplify to 4.1.1 (with the fix brix/crypto-js#364 on removing dependency for node.js core modules "crypto"), so the breaking change error should be gone now.

Note we still need to add the following config to acknowledge the commonJS dependencies

 "allowedCommonJsDependencies": [
      "buffer",
      "crypto-js",
      "isomorphic-unfetch"
    ],

I will close the issue for now, but let me know if you have further question.

Issues Triaging automation moved this from Feature Requests to Triaged/Closed By DSE Jul 27, 2021
@rajinder-yadav
Copy link
Author

@hkjpotato great work, thanks for the fix!

@github-actions
Copy link

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Angular Related to Angular 2+ feature-request Request a new feature
Projects
No open projects
Issues Triaging
Triaged/Closed By DSE
Development

No branches or pull requests

3 participants