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 npm with github #160

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tjenkinson
Copy link

so that users can

npm install --save github:NeilFraser/JS-Interpreter

closes #156

See https://docs.npmjs.com/cli/install

I wondered if it would work without name but it's required. It's a bit misleading if someone else owns js-interpreter on npm. Ideally you would register a new name and use that to prevent any confusion.

so that users can

```
npm install --save github:NeilFraser/JS-Interpreter
```

closes NeilFraser#156
@tjenkinson tjenkinson marked this pull request as ready for review August 31, 2019 13:33
@cpcallen
Copy link
Collaborator

cpcallen commented Sep 1, 2019

I'm not clear on how to contact the person who currently owns the js-interpreter npm package, nor whether they would be willing and able to transfer it, but I would say it would be preferable for that npm to be owned by @NeilFraser, and for this PR to be accepted in support of having an official version whether under that name or another.

@@ -0,0 +1,17 @@
{
"name": "js-interpreter",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about adding a scope, that would allow the distinction to the existing package: @neil-fraser/js-interpreter or something along those lines?

https://docs.npmjs.com/misc/scope

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on the scoping. Is that @NeilFraser 's npm account?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that @NeilFraser 's npm account?

I rather doubt that @NeilFraser has an npm account. I might be guilty of understatement in describing him as being a bit dubious about the npm ecosystem. (I still think it would be preferable if he make his packages officially available there, though.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If he registers an account, even if he doesn't publish there, it would be better because we could use a scoped name that no one else could take then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be guilty of understatement in describing him as being a bit dubious about the npm ecosystem

Hopefully things have changed now in 2021 :D

@hsluoyz
Copy link

hsluoyz commented Dec 22, 2019

Any update?

@mercmobily
Copy link

I'm not clear on how to contact the person who currently owns the js-interpreter npm package, nor whether they would be willing and able to transfer it, but I would say it would be preferable for that npm to be owned by @NeilFraser, and for this PR to be accepted in support of having an official version whether under that name or another.

Please see my other ticket. #216 I will be totally happy to transfer ownership of it to you guys. But, I think we should spare a thought for those people who are currently using npm's js-interpreter so that we don't break things for them.

@mercmobily
Copy link

I had a look at this PR, and I wanted to add that doing this might not be enough:

if (typeof exports === 'object' && typeof module === 'object') {
  module.exports = Interpreter;
}

Mainly because of this line in js-interpreter:

Interpreter.nativeGlobal = this;

Sandwiching it might be a better option, since this will change its meaning depending on where it's being used. However, I am not the strongest JS person in my team. Raphael (in my team) will do a much better job than me at checking these sort of issues.

@cpcallen
Copy link
Collaborator

I'm on annual leave at the moment but will look at this when I get back to the office. Thanks @tjenkinson for the PR and @mercmobily for offering the use of the existing name. I'm not sure that @NeilFraser will be that keen to take on maintenance of the npm but we can probably work something out, especially if there are folks like you guys with the relevant experience who are willing to help out.

I agree that it would be best not to break things for users of the existing package.

@mercmobily
Copy link

No worries, let me know once you get back. In the meantime, I will leave the current js-interpreter npm package as is, waiting for your decision!
Thanks for taking the time to answer during your annual leave.

@derwehr
Copy link

derwehr commented Mar 14, 2022

Any chance this PR gets merged?
And is there currently another solution to install js-interpreter as a npm dependency?

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

Successfully merging this pull request may close these issues.

Adding package.json
6 participants