dcodeIO / bcrypt.js

Optimized bcrypt in plain JavaScript with zero dependencies.
Other
3.47k stars 264 forks source link

module and process variables #121

Open mdhishaamakhtar opened 3 years ago

mdhishaamakhtar commented 3 years ago

The variables module and process in the file minimal-env.js is overriding the definition of module and process in Node.js

dcodeIO commented 3 years ago

minimal-env.js isn't actual code, that's a closure compiler externs file. Can you explain the problem you are facing in more detail?

mdhishaamakhtar commented 3 years ago

When I am using any sort of IDE, its taking minimal-env.js as the global declarations for module and process. Hence, resulting in errors when I do module.exports or process.env.PORT for example.

dcodeIO commented 3 years ago

That's strange, I've never seen a problem like this using any sort of IDE I use. Given that it's not feasible to delete the file as long as we want to use it with closure compiler, I'm also not sure what to do here.

mdhishaamakhtar commented 3 years ago

I have not been through the codebase entirely and I intend to do that, but is it necessary to name those variables as process and module?

dcodeIO commented 3 years ago

Yeah, that's necessary, since the file indicates the expected environment when compiling with closure compiler, to make it aware of it, so it doesn't rename the wrong things during optimizations. More information about externs: https://developers.google.com/closure/compiler/docs/externs-and-exports

mdhishaamakhtar commented 3 years ago

Alright then should I put it in .npmignore and make a PR because we dont need the npm package to have that since its only needed for the closure compiler?

mdhishaamakhtar commented 3 years ago

Alright I have opened a PR for the same.

mdhishaamakhtar commented 3 years ago

Is there any update @dcodeIO

nowm commented 3 years ago

@dcodeIO Thank you for your great efforts on creating and supporting this awesome package, but this minimal-env.js problem is really annoying and breaks IDE things after installing bcryptjs (in my case any IDEA-family IDE, like WebStorm, PhpStorm, PyCharm and so on).

Any chances to have an update?

As a workaround for people coming here from Google search: right click on [project root]/node_modules/bcryptjs/externs/minimal-env.js => "Mark as Plain Text". See also a discussion in #109.

mdhishaamakhtar commented 3 years ago

I think with #124 merged into master, we can close this issue now

nowm commented 3 years ago

@mdhishaamakhtar and @dcodeIO, thank you so very much for this fix! Hope to see updates in NPM soon.

mdhishaamakhtar commented 3 years ago

Since #124 is still not added in the latest release, I'll let this stay open until there is a new release for npm

mdhishaamakhtar commented 3 years ago

@dcodeIO If you could publish a new release to npm, then we could close this issue.

VladStepanov commented 3 years ago

@dcodeIO any updates about release?