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

Support for process.env? #32

Closed
MarcusRiemer opened this issue Aug 2, 2022 · 4 comments
Closed

Support for process.env? #32

MarcusRiemer opened this issue Aug 2, 2022 · 4 comments

Comments

@MarcusRiemer
Copy link
Contributor

I have a JavaScript Library, namely immer.js that I would love to run in goja. Judging by the looks of it immer prefers to work with Proxy objects, but can explicitly made ES5 compatible. It's default index.js however expects to be run in a node.js compatible environment:

'use strict'
if (process.env.NODE_ENV === 'production') {
  module.exports = require('./immer.cjs.production.min.js')
} else {
  module.exports = require('./immer.cjs.development.js')
}

This causes a ReferenceError: process is not defined at main.js:5135:7(1) when I try to load a file that has been put together by Rollup.

I'm explicitly not asking to implement all of the node Process API, only process.env. From my (very limited) understanding this would require an object named process with a env property to be made available in the global namespace. The values could probably be retrieved from os.Environ().

Would such a PR be accepted and a good "first" kind of issue to explore the codebase?

@dop251
Copy link
Owner

dop251 commented Aug 2, 2022

Sure. You can use the console module as an example.

BTW, goja supports Proxy (and many other ES6 features)

@MarcusRiemer
Copy link
Contributor Author

MarcusRiemer commented Aug 4, 2022

Wow, I just wanted to say that implementing this was a very pleasant experience. I feared that there was lots of marshaling awaiting me, but at least the very minimal implementation to fulfill my use case worked out of the box. I deliberately added an unconditional fail to the tests because I couldn't believe how it just worked.

And I am also in awe how far the ES6 support is by now. This is great news!

@kke
Copy link

kke commented Sep 30, 2022

The "process.env" part from #33 was already merged, so in that regard this issue has been solved.

I think process.stdout.write and process.stderr.write("foo\n") should be rather easy to add and there are other low hanging fruits in there that should be pretty much oneliners. Maybe taking a stab at those at some point unless someone is faster.

@MarcusRiemer
Copy link
Contributor Author

Sorry, forgot to close the issue. Feel free to go for those stderr and stdout parts, I personally do not need them and don't plan on tackling them. I'm not 100% confident that they are oneliners because I have no idea how they would / should behave when called with binary data.

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

3 participants