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 directly #489

Open
graingert opened this issue May 24, 2016 · 16 comments
Open

support NPM directly #489

graingert opened this issue May 24, 2016 · 16 comments

Comments

@graingert
Copy link
Contributor

It would be awesome if we could port the .bat or .py to JavaScript so that we can distribute this on npm directly.

@timdream
Copy link

@graingert I wonder what "support NPM directly" means? Your wrapper package will be still very useful in terms of download the specific version of XULRunner/Firefox for SlimerJS. It's not that Mozilla publishes Firefox binary on NPM, nor SlimerJS v0.10 work on every Firefox version out there.

Are you thinking about achieving feature parity by having NPM install script specified by package.json to download Firefox v38 (currently, matching 0.10) in this repo?

Thanks!

@graingert
Copy link
Contributor Author

@timdream SlimerJS no longer uses xulrunner and is just a shell script wrapper around Firefox. This can be ported to JavaScript and released on npm

@timdream
Copy link

@graingert You are right. I was just trying to figure out the desired outcome.

Yet, I failed to see the reason of porting it to JavaScript as a requirement; there is no restriction on what you could publish on NPM. The slimerjs package interfaces with NodeJS with child process and it's fine. The benefit of porting it to JavaScript is probably allow script-able access from NodeJS, which is desirable but shouldn't be blocking anything.

We could, and probably should upgrade slimerjs package to contain the SlimerJS itself. Yet, to make npm install slimerjs work as before, the command has to download Firefox for the given OS as well. Is that something you believe should be done in this repo too?

This issue is not actually actionable only If that's the case and @laurentj agrees.

@KevinGrandon
Copy link
Contributor

I believe that the desired outcome is a replacement for the existing npm package: https://www.npmjs.com/package/slimerjs

E.g., a npm package which will download slimerjs as well as a copy of Firefox. That way you can use it within projects by just adding a line to package.json.

@graingert
Copy link
Contributor Author

I don't think it should download Firefox, just put a package.json in the
root of this repo

On 26 Jul 2016 00:01, "Kevin Grandon" notifications@github.com wrote:

I believe that the desired outcome is a replacement for the existing npm
package: https://www.npmjs.com/package/slimerjs

E.g., a npm package which will download slimerjs as well as a copy of
Firefox. That way you can use it within projects by just adding a line to
package.json.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#489 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZQTPLSmfTzODxsBkRzgEZWGGOpb8UEks5qZUA0gaJpZM4IlSjT
.

@KevinGrandon
Copy link
Contributor

@laurentj - Would you consider accepting contributions to add NPM support? The primary change being just a package.json so we can download the scripts via NPM.

@graingert
Copy link
Contributor Author

Also a port of the python/shell code to JavaScript

On 26 Jul 2016 00:17, "Kevin Grandon" notifications@github.com wrote:

@laurentj https://github.com/laurentj - Would you consider accepting
contributions to add NPM support? The primary change being just a
package.json so we can download the scripts via NPM.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#489 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZQTO48QUalQC3Uq8DlydNaOvk4tAxuks5qZUQYgaJpZM4IlSjT
.

@timdream
Copy link

Also a port of the python/shell code to JavaScript

@graingert I don't understand this part and you never replied my comment above. I would doubt this is a requirement to make this repo supports NPM.

@timdream
Copy link

I don't think it should download Firefox, just put a package.json in the
root of this repo

It would be an issue for locking down the right Firefox version for the specific version of the SlimerJS. Not a problem for Travis-CI users – version can be set and done in .travis.yml, but it would be a bottleneck for projects that welcome contributions (and expects contributors to run tests locally everywhere.)

@KevinGrandon
Copy link
Contributor

I'm working on a PoC which will download both SlimerJS as well as a copy of Firefox and have both installed into the node_modules/ folder. If I can get everything working I'll see about upstreaming pieces here or into the existing slimerjs npm repo.

https://github.com/laurentj/slimerjs/compare/master...KevinGrandon:master?expand=1

@timdream
Copy link

@KevinGrandon Upon seeing your patch, I was going to say download the Firefox binary itself should be a package of it's own, e.g. mozilla-download. But it pull from the Mozilla CI (TaskCluster) instead of the release build archive; it would take some efforts to update it to do so (and have SlimerJS npm install script set the right version of Firefox that maps to the version of it's own).

I did see value in keep maintaining mozilla-download; not sure about what you think though.

@KevinGrandon
Copy link
Contributor

Yeah, that fork is still WIP and I'm evaluating the best way to move forward. After playing with it a bit I do think it would be nice to have some way in package.json to declare whether or not to actually download Firefox. Often some CI solutions like Travis allow you to install Firefox as an addon. It would be nice if slimer could work with a local copy, or a global copy of Firefox.

Another package for downloading Firefox would be fine, I'll keep looking into this.

@graingert
Copy link
Contributor Author

Downloading firefox is really not needed. People should just install it from their repos or firefox.com

@graingert graingert reopened this Aug 5, 2016
@KevinGrandon
Copy link
Contributor

Downloading firefox is really not needed. People should just install it from their repos or firefox.com

The reason I think that the npm package should download firefox is because it's not always compatible with the latest firefox version. Right now for example the repo does not work with the latest verion (v47/48).

Having an easy way for developers to leverage the functionality without them having to download another compatible copy of Firefox is a big when.

Previously:
Only use slimerjs npm package.

Now:
Use slimerjs npm package once support is added.
Download a compatible version of Firefox.
Use the Env var to specify the location of the compatible version.

There's a lot more boilerplate compared to before and it would be nice if we give them an easier solution.

@graingert
Copy link
Contributor Author

Let's just get an npm package that works for people who have Firefox
installed first

On 5 Aug 2016 16:03, "Kevin Grandon" notifications@github.com wrote:

Downloading firefox is really not needed. People should just install it
from their repos or firefox.com

The reason I think that the npm package should download firefox is because
it's not always compatible with the latest firefox version. Right now for
example the repo does not work with the latest verion (v47/48).

Having an easy way for developers to leverage the functionality without
them having to download another compatible copy of Firefox is a big when.

Previously:
Only use slimerjs npm package.

Now:
Use slimerjs npm package once support is added.
Download a compatible version of Firefox.
Use the Env var to specify the location of the compatible version.

There's a lot more boilerplate compared to before and it would be nice if
we give them an easier solution.


You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub
#489 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZQTHppu-VjEt7Nl8xDkJrPcBtG2CjVks5qc1DBgaJpZM4IlSjT
.

@yairEO
Copy link

yairEO commented Oct 30, 2016

Right now for example the repo does not work with the latest verion (v47/48).

That isn't the case now with 0.10.1 ("Compatibility with Firefox 47, 48, 49")

I agree downloading Firefox manually and installing on the test machine does sucks very much and feels "wrong". There should have been a head-less version of FF just for testing things. what a shame.

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