search

dop251 / goja_nodejs

Nodejs compatibility library for Goja
MIT License
337 stars 81 forks source link

Minimal implementation to provide `process.env` #33

Closed MarcusRiemer closed 2 years ago

MarcusRiemer commented 2 years ago

As discussed in #32: This is a very minimal "implementation" of process.env to provide data for exactly this use case. I fear this will be incompatible if anyone attempts to build a "real" implementation of the node.js process API, so I deliberately named the plugin process.env.

dop251 commented 2 years ago

I think the module should be called process since it sets the process property. It's still better than the current alternative (which is none) and if someone wants a more involved implementation they will just change it.

MarcusRiemer commented 2 years ago

Fair enough, I updated the PR.

dop251 commented 2 years ago

Could you please also make sure that the Windows test does not fail on CommonProgramFiles(x86)? Environment variable names can contain special characters so you can't use them in a dot expression.

MarcusRiemer commented 2 years ago

Ah sorry, somehow I overlooked the failing checks. I removed the offending tests using the dot-operator as it is rather unlikely that there could go some wrong on such a fundamental level. I instead added a test for an explicitly set environment variable that is unlikely to be encountered in the CI pipeline naturally.