Outdated buffer sub dependency causes `global is not defined` error

[ohcibi]

🐞 Bug report

Following #8160 I found that it is still possible to cause the global ist not defined error without installing a library that actively uses global. It is caused by buffer which is a sub dependency of @angular-devkit/build-angular.

Command (mark with an x)

No command. It is a runtime issue. Any command that builds the project obviously.

Is this a regression?

Yes, the regression is obviously the removal of the global shim.

Description

@angular-devkit/build-angular depends on a version of webpack, which depends on a version of node-libs-browser (deprecated!) which depends on a version of buffer that is actually using global.

$ npm ls buffer
iotgen-webui@0.1.0 /Users/ohcibi/Documents/Projekte/PERK/iotgen/iotgen-webui
└─┬@angular-devkit/build-angular@0.803.14
  └─┬ webpack@4.39.2
    └─┬ node-libs-browser@2.2.1
      └── buffer@4.9.1

The latest versions of the buffer package have fixed that but since node-libs-browser is deprecated, this won't fix on webpack4. webpack 5 does not have the node-libs-browser dependency anymore.

The library I have installed and try to import was jwt-simple.

🔬 Minimal Reproduction

npm install -D jwt-simple
# Add `import jwt from 'jwt-simple' to some angular module
ng serve
# Visit route that uses the module with the import and see error in console

🔥 Exception or Error

index.js:43 Uncaught ReferenceError: global is not defined
    at Object../node_modules/buffer/index.js (index.js:43)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/safe-buffer/index.js (index.js:2)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/randombytes/browser.js (browser.js:15)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/crypto-browserify/index.js (index.js:3)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/jwt-simple/lib/jwt.js (jwt.js:13)
    at __webpack_require__ (bootstrap:79)

🌍 Your Environment

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / ‚ñ≥ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 8.3.12
Node: 12.12.0
OS: darwin x64
Angular: 8.1.3
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.803.14
@angular-devkit/build-angular     0.803.14
@angular-devkit/build-optimizer   0.803.14
@angular-devkit/build-webpack     0.803.14
@angular-devkit/core              8.3.14
@angular-devkit/schematics        8.3.12
@angular/cli                      8.3.12
@ngtools/webpack                  8.3.14
@schematics/angular               8.3.12
@schematics/update                0.803.12
rxjs                              6.4.0
typescript                        3.4.5
webpack                           4.39.2
    

[filipesilva]

Interesting. Wasn't expecting any of the node libraries to be included since we set the node option in webpack to false for browser builds.

But maybe webpack special-cases crypto as it's also a browser API? I expected it to just leave it as window.crypto or something.

I get the feeling we'll have to wait for Webpack 5 to address this.

[clydin]

crypto is a Node.js specific API. Newer browsers contain the Web Cryptography API (https://www.w3.org/TR/WebCryptoAPI/#introduction). While they both provide cryptography functionality, they are vastly different.

It is also important to note that enabling any of the Webpack node shims/polyfills is considered unsupported behavior. Alternatively, if using the path mapping method, the developer will need to ensure that all requirements of the path mapped shim are also covered within the application. It is highly recommended to use packages that truly support the browser platform or are cross-platform to avoid issues such as this. And related to this, Webpack 5 will default to disabling all node shimming.

[ohcibi]

That might all be true but the purpose of this issue is that the information that’s spread out about that error is not accurate. What you find is that it’s the libraries authors and thus the choice of the developer to use that libraries fault that it happens. But in this case the library is not doing anything deprecated but the broken shim that webpack 4 depends on and won’t change that is. Ironically while being a browser shim itself. So for this instance of this error it’s a simple regression that is caused by removing the shims from angular-cli while webpack wasn’t ready for that. Clearly this removal should have waited for webpack removing that broken dependency and the information about that situation needs to be updated instead of simply blaming other library authors to do deprecated stuff (after all the deprecated dependency causing trouble here comes from angular-cli itself).

[clydin]

The library in question is using “crypto” which does not exist in a browser and as a result is not a browser compatible library. Using shims or polyfills to attempt to work around the issue is and will always be complex and problematic. Especially in this case as it appears that the shims themselves require shims making them not cross platform as well. Libraries that are not cross platform or web specific should be avoided when possible. Not only do issues such as this result but it can also cause performance and size concerns for the application.

In regards to “crypto” itself in this context, It is highly recommended to use built in and fully validated cryptographic implementations. This is especially important when related to security and authorization/authentication use cases. On a browser that would be the Web Cryptography API.