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

Get error using nats.ws and angular 17 #241

Closed
fclmman opened this issue May 17, 2024 · 13 comments
Closed

Get error using nats.ws and angular 17 #241

fclmman opened this issue May 17, 2024 · 13 comments
Labels
defect Suspected defect such as a bug or regression

Comments

@fclmman
Copy link

fclmman commented May 17, 2024

Observed behavior

get error building project
Could not resolve "crypto"

node_modules/nats.ws/esm/nats.js:4913:24:
  4913 │       crypto1 = require('crypto');
       ╵                         ~~~~~~~~

The package "crypto" wasn't found on the file system but is built into node. Are you trying to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

Expected behavior

project builds without error

Server and client version

"nats.ws": "^1.26.0",
"@angular": "^17.3.0",

Host environment

No response

Steps to reproduce

create project with ng new, add nats.ws create connection and try to build

@fclmman fclmman added the defect Suspected defect such as a bug or regression label May 17, 2024
@aricart
Copy link
Member

aricart commented May 21, 2024

There's some bundler configuration that is happening here - the nats.ws/esm/nats.js doesn't have a single require there as shipped.

I believe your bundler is picking up nats.ws/cjs/nats.js which will have require this is only useful if you have a shim for node that does w3c sockets and will never ever work in the browser. Make sure you are not doing any require when importing the nats.ws library.

It should be import {connect} from "node_modules/nats.ws/esm/nats.js"

@aricart
Copy link
Member

aricart commented May 21, 2024

You may want to extract the "node_modules/nats.ws/esm/nats.js" file and reference it directly - depending on your bundler it may not correctly select between cjs and esm.

@aricart aricart closed this as completed May 21, 2024
@nickchomey
Copy link

nickchomey commented Jun 3, 2024

I get similar issues when I try to run nats.js through esbuild in order to minify and tree shake it.

It stems from the require('crypto') here https://github.com/nats-io/nkeys.js/blob/88d9d1c6479755ab4a215a7d5265b9d01990c57e/modules/esm/nacl.js#L1172C6-L1172C32

It seems that esbuild is trying to resolve this, but since I am building for the browser, it fails. If I build for nodejs, it succeeds, but that's not what I want.

If you wrap the require in a try.. catch, such as the following, it will work

} else if (typeof require !== 'undefined') {
    try {
        crypto1 = require('crypto');
        if (crypto1 && crypto1.randomBytes) {
            nacl.setPRNG(function(x, n) {
                var i, v = crypto1.randomBytes(n);
                for (i = 0; i < n; i++) x[i] = v[i];
                cleanup(v);
            });
        }
    } catch (e) {
        console.log('crypto module is not available in this environment');
    }
}

If you do a dynamic import, it works as well.

import('crypto')
.then(crypto => {
    if (crypto && crypto.randomBytes) {
        nacl.setPRNG(function(x, n) {
            var i, v = crypto.randomBytes(n);
            for (i = 0; i < n; i++) x[i] = v[i];
            cleanup(v);
        });
    }
})
.catch(err => {
    console.log('crypto module is not available in this environment');
});

I tried building with bun, and it seems to work regardless, but that's because it imports the entire node:crypto module into the bundle. Neither try..catch or dynamic import seems to affect this.

Would it be possible to add one of these solutions, so that at least esbuild (and whatever OP was using) will work?

Its a similar issue to what I reported here https://github.com/nats-io/nats.js/issues/608#issuecomment-1847334882

Side note - Deno bundle has been deprecated, and it doesnt have minify anyway. Perhaps something else (e.g. bun) could be used instead?

@aricart
Copy link
Member

aricart commented Jun 3, 2024

@nickchomey bundle is now an external api, it is still supported - with that said, I am in the middle of an extensive refactoring for the clients, which will split the lib into several - core, jetstream, kv, object store and services. Each will have an esm only option or a node option - the ESM only (via jsr) will do exactly what you want as it will only involve webapis.

@nickchomey
Copy link

Perfect! In the meantime I've forked the repo and applied my changes.

@playerx
Copy link

playerx commented Aug 25, 2024

Hi guys, still having the same issue with Angular 18:

✘ [ERROR] Could not resolve "crypto"

    node_modules/nats.ws/esm/nats.js:4911:24:
      4911 │       crypto1 = require('crypto');

@nickchomey can you share the forked version please? Or maybe there is other solution?

@aricart I checked, but didn't find nats.ws on jsr, can you share if it's there? maybe with other name?

thanks 🙌🏻

@aricart
Copy link
Member

aricart commented Aug 25, 2024

The esm lib shouldn't have a require, as that is not available in browser, so all browser apps would fail. Shims for crypto should be available on npm...

@playerx
Copy link

playerx commented Aug 26, 2024

@aricart Yes I agree, I think issue is that nats.ws loads crypto.

I've created a simple project to reproduce the issue:
https://github.com/playerx/nats-issue

Steps I did:

  1. Create a new angular with ng new command
  2. Install and use nats.ws by running: npm i nats.ws

And npm run build fails with the error I mentioned.

If there is a way to avoid loading crypto please let me know.

@playerx
Copy link

playerx commented Aug 26, 2024

Not sure if installing shim is the right way to go. I think the main problem is that it loads crypto when it shouldn't.

@playerx
Copy link

playerx commented Aug 26, 2024

Workaround: Adding the following in package.json did the trick:

"browser": {
    "crypto": false
  }

However, I think for the long-term solution nats.ws shoudn't be loading crypto in browser.

@aricart
Copy link
Member

aricart commented Aug 26, 2024

Crypto is required for objectsstore, and works perfectly in browser...

https://developer.mozilla.org/en-US/docs/Web/API/Crypto

@aricart
Copy link
Member

aricart commented Aug 26, 2024

The 3.0 version of the clients breaks the lib into several. Crypto dependency will be on the objectstore module only
Stay tuned

@jymukendi
Copy link

Hi guys, I am also getting the same issue in the latest Angular 18. ng build is throwing the same error:
Screenshot 2024-10-16 at 08 50 47

@playerx's workaround got rid of it, but that should get fixed in nats.ws.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

No branches or pull requests

5 participants