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

angular-lib build path doesn't work with mqtt as dependency #156

Closed
sclausen opened this issue Aug 11, 2020 · 10 comments · Fixed by #158
Closed

angular-lib build path doesn't work with mqtt as dependency #156

sclausen opened this issue Aug 11, 2020 · 10 comments · Fixed by #158

Comments

@sclausen
Copy link
Owner

@nosovk Unfortunately, the published package is broken, since mqtt can't be embedded without browserification.
Do you have a solution for this issue, or know how to embed the old vendor mqtt.js?

That's the error I get when using the library in 7.0.9.

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/readable-stream/node_modules/safe-buffer/index.js (index.js:2)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/readable-stream/lib/_stream_readable.js (_stream_readable.js:55)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/readable-stream/readable-browser.js (readable-browser.js:1)
    at __webpack_require__ (bootstrap:79)
    at Object../node_modules/mqtt/lib/store.js (store.js:8)
    at __webpack_require__ (bootstrap:79)
@nosovk
Copy link
Contributor

nosovk commented Aug 12, 2020

@AlxZchk how you use it now?
#153 this one is landed, you can use npm version instead of stiking to PR, check pls

@nosovk
Copy link
Contributor

nosovk commented Aug 12, 2020

@nosovk
Copy link
Contributor

nosovk commented Aug 12, 2020

nodejs/readable-stream#435
looks like they started to do similar work that I do in mqttjs, removing all globals. But... I'm not sure that it's possible for package like that

@nosovk
Copy link
Contributor

nosovk commented Aug 12, 2020

good part is that in webpack 5 node shims will be removed from webpack, and all libs will have to avoid using them, but till then we need some fix. To be clear I have one super hack solution now. But it's pretty ugly.

@nosovk
Copy link
Contributor

nosovk commented Aug 12, 2020

npm after install script:

const fs = require('fs');
const f = 'node_modules/@angular-devkit/build-angular/src/angular-cli-files/models/webpack-configs/browser.js';

fs.readFile(f, 'utf8', function (err,data) {
  if (err) {
    return console.log(err);
  }
  // at some moment angular-cli tam disabled node polyfills and stubs in webpack
  // from:
  // node: false
  // to:
  // node: {global: true}
  const result = (data.replace(/node: false/g, "node: {global: true}"));
  fs.writeFile(f, result, 'utf8', function (err) {
    if (err) return console.log(err);
  });
});

this will hepl, but it's a super dirty solution, should be avoided in any lib (but it works)

@sclausen
Copy link
Owner Author

Is this hack enough to run mqtt in ngx-mqtt or does it just work for the specific global issue?

@nosovk
Copy link
Contributor

nosovk commented Aug 12, 2020

yes. This hack will fix it.
To clarify it would work even without publishing my fixes to mqtt (they merged, but not published)

@AlxZchk
Copy link

AlxZchk commented Aug 12, 2020

I'm using 7.0.9 version in angular project.
But I had to add these to lines to polyfills.ts to make it work.

global.Buffer = global.Buffer || require('buffer').Buffer;
global.process = require('process');

@nosovk
Copy link
Contributor

nosovk commented Aug 12, 2020

ok, my hack will do almost the same, but using webpack shim (will broke on webpack 5, where shims will be removed). Webpack 5 in roadmap of Angular development. Currently hack will work, please make PR with it and reference it here.

@nosovk
Copy link
Contributor

nosovk commented Aug 18, 2020

@sclausen could you please review\merge that PR? It will fix broken builds.

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

Successfully merging a pull request may close this issue.

3 participants