removing node-shims in v6.0.0 breaks ngx-mqtt

[sclausen]

I understand the motivation of removing node-shims, but it breaks my library and I currently don't know how to work around this.

Versions

Angular CLI: 6.0.0-rc.5
Node: 8.11.1
OS: linux x64
Angular: 6.0.0-rc.3
... animations, common, compiler, compiler-cli, core, forms
... http, platform-browser, platform-browser-dynamic, router

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.5.4
@angular-devkit/build-angular      0.5.4
@angular-devkit/build-ng-packagr   0.5.4
@angular-devkit/build-optimizer    0.5.4
@angular-devkit/core               0.5.4
@angular-devkit/schematics         0.5.7
@angular/cli                       6.0.0-rc.5
@ngtools/json-schema               1.1.0
@ngtools/webpack                   6.0.0-rc.2.4
@schematics/angular                0.5.7
@schematics/update                 0.5.7
rxjs                               6.0.0-tactical-rc.1
typescript                         2.7.2
webpack                            4.5.0

Repro steps

Running an application with a ngx-mqtt@6.0.0-alpha.8 build of my library results in an error that process is not defined.

• clone https://github.com/sclausen/ngx-mqtt-next
• run it with ng serve
• open the console and see the error

Observed behavior

index.js:3 Uncaught ReferenceError: process is not defined
    at Object../node_modules/process-nextick-args/index.js (index.js:3)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/readable-stream/lib/_stream_readable.js (_stream_readable.js:26)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/readable-stream/readable-browser.js (readable-browser.js:1)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/mqtt/lib/store.js (store.js:8)
    at __webpack_require__ (bootstrap:74)
    at Object../node_modules/mqtt/lib/client.js (client.js:7)
    at __webpack_require__ (bootstrap:74)
./node_modules/process-nextick-args/index.js @ index.js:3
__webpack_require__ @ bootstrap:74
./node_modules/readable-stream/lib/_stream_readable.js @ _stream_readable.js:26
__webpack_require__ @ bootstrap:74
./node_modules/readable-stream/readable-browser.js @ readable-browser.js:1
__webpack_require__ @ bootstrap:74
./node_modules/mqtt/lib/store.js @ store.js:8
__webpack_require__ @ bootstrap:74
./node_modules/mqtt/lib/client.js @ client.js:7
__webpack_require__ @ bootstrap:74
./node_modules/mqtt/lib/connect/index.js @ index.js:3
__webpack_require__ @ bootstrap:74
./node_modules/ngx-mqtt/esm5/ngx-mqtt.js @ validations.js:52
__webpack_require__ @ bootstrap:74
./src/app/app.component.ts @ main.js:72
__webpack_require__ @ bootstrap:74
./src/app/app.module.ts @ app.component.ts:15
__webpack_require__ @ bootstrap:74
./src/main.ts @ environment.ts:16
__webpack_require__ @ bootstrap:74
0 @ main.ts:12
__webpack_require__ @ bootstrap:74
checkDeferredModules @ bootstrap:43
webpackJsonpCallback @ bootstrap:30
(anonymous) @ main.js:1

Desired behavior

I would like to either optionally add node-shims or maybe a possibility to add browserified code to the bundle. I added mqtt directly to my library dependencies and since mqtt provides a browserified bundle, this would be an option too. But ng-packagr currently has no option to directly embedd javascript files into the bundle.

Mention any other details that might be useful (optional)

ngx-mqtt has a branch 6.0.0-alpha with the library code.

[clydin]

Libraries that support multiple platforms (node and browser, in this case) should not assume the presence of platform specific APIs. This is an issue that should be corrected within the library itself, if it is truly meant to be supported within the browser. For further details, please see this comment: #9827 (comment)

Also, as a last resort and if correcting the library is a non-starter, then there also appears to be a node process shim that could be experimented with here: https://github.com/defunctzombie/node-process

[sclausen]

So every dependency which has worked back then has to be rewritten? Seems a bit extreme. You're breaking a lot of stuff without any replacement (for example adding browserified javascript to the bundles would be an option).

[clydin]

Not rewritten just made actually cross-platform (node and a browser are different platforms). This is realistically no different than a C program that needs to support both Windows and Linux.
Generally, this requires minimal changes to the libraries (e.g., safety checks on use of process.env). Library documentation listing the necessary shims and/or polyfills could also be a solution. The browser property in package.json is also a useful option in some cases.
Also, as mentioned in the linked comment, there are multiple technical issues alone with the previous approach.

[sclausen]

I never said that this change should've not been made! Again, I totally understand that decision. It's just way more sophisticated to fix the now broken dependencies than you connote. In my mqtt.js case, this would mean to also fix some of their dependencies aswell. Doing this, then waiting for the pull requests being accepted, merged and released would be a hell lot of work and time.

Maybe almost no classic angular-cli frontend projects currently have such severe issues with npm packages and it's just because I tried to change the build tool of my library from manual webpack and browserify to an angular-cli library project. So thanks for your time, maybe using angular-cli as a build tool isn't the right choice for me.

[dopry]

As a consumer of ngx-mqtt, it would be nice to have introduced a deprecation period, so upstream developers could address the issues. It's nice when a project telegraphs it's wider intentions so the broader community can keep pace and deliver a better developer experience.

[sclausen]

@dopry If you use ngx-mqtt >= 6 with angular >= 6, you're safe, because I didn't use mqtt as a npm dependency anymore. angular 5 with ngx-mqtt < 6 should still work with angular-cli.

[dopry]

@sclausen, got it. Thanks.

[hassan-yonomi]

@sclausen i just recently ran into this issue while trying to integrate aws-iot-device-sdk with my angular 6 app. This is how i got around this "ReferenceError: process is not defined" issue.

Added process module via npm
"process": "^0.11.10"

Added this to polyfill.ts file
(window as any).process = require('process/browser');

This fixes the issue for me, not an ideal solution though. Would love to see a better fix

[IAMtheIAM]

@hassan-yonomi You sir, are the man! I struggled with this for weeks, your solution immediately worked. I wonder what caused this breaking change? Thank you either way.

[sclausen]

@hassan-yonomi Wow, this seems like a nice workaround. Maybe I'll try again with angular-cli.

[opiepj]

@clydin Just curious, how did Angular 5 insert their node shims before? Maybe other projects could mimic that?

[hassan-yonomi]

@clydin i think angular-cli added the shims during the build process. Initially i started to research adding the shims back but my search hit a lot of dead-ends. I think the fix needs to be on the MQTT lib that requires node native functionality for browser side implementations.

[sclausen]

@hassan-yonomi unfortunately I didn't found a way to include polyfills.ts to an angular-cli library project 😞 And the dependant libraries don't get bundled with my library, so it's still not an option for me to use angular-cli.