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

module and process variables #121

Open
mdhishaamakhtar opened this issue Sep 22, 2020 · 14 comments
Open

module and process variables #121

mdhishaamakhtar opened this issue Sep 22, 2020 · 14 comments

Comments

@mdhishaamakhtar
Copy link
Contributor

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

@dcodeIO
Copy link
Owner

dcodeIO commented Sep 22, 2020

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
Copy link
Contributor Author

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
Copy link
Owner

dcodeIO commented Sep 23, 2020

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
Copy link
Contributor Author

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
Copy link
Owner

dcodeIO commented Sep 23, 2020

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Alright I have opened a PR for the same.

@mdhishaamakhtar
Copy link
Contributor Author

Is there any update @dcodeIO

@nowm
Copy link

nowm commented Nov 5, 2020

@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
Copy link
Contributor Author

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

@nowm
Copy link

nowm commented Dec 4, 2020

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

@mdhishaamakhtar
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@VladStepanov
Copy link

VladStepanov commented Dec 30, 2020

@dcodeIO any updates about release?

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

No branches or pull requests

4 participants