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

Mithril's Bundler overrides Ruby Bundler in 'npm run' scripts #1668

Closed
illarionvk opened this issue Mar 1, 2017 · 11 comments
Closed

Mithril's Bundler overrides Ruby Bundler in 'npm run' scripts #1668

illarionvk opened this issue Mar 1, 2017 · 11 comments
Milestone

Comments

@illarionvk
Copy link

I'm working on an app that will be hosted on Github Pages. I use github-pages gem to compile the static site with Jekyll locally.

I use npm scripts in package.json to run the build task:

//...
  "scripts": {
    "build": "bundle exec jekyll build",
  }
//...

The bundle exec command should launch Ruby Bundler gem, but it launches Mithril's Bundler instead. This happens because npm scripts look up executables in node_modules/.bin folder first.

Ruby Bundler is far more popular than Mithril's Bundler. Would you consider renaming the executable in .bin folder from bundle to mithril-bundle?

@illarionvk
Copy link
Author

For now I'm using a postinstall script to remove Mithril's bundle symlink from node_modules/.bin:

//…
  "scripts": {
    "postinstall": "rm -f node_modules/.bin/bundle",
    //…
  }
//…

@barneycarroll
Copy link
Member

@illarionvk so installing Mithril as an NPM dependency adds Mithril's bundle NPM script to the node scripts of your dependent package? That seems like broken NPM behaviour, surely?

@illarionvk
Copy link
Author

@barneycarroll Installing Mithril as an NPM dependency adds a symlink of Mithril's bundle executable to ./node_module/.bin/ folder:

mithril-bundle-symlink

@pygy
Copy link
Member

pygy commented Mar 1, 2017

@barneycarroll I think it is intentional in order to allow meta-packages of commands.

I'm surprised there's no way to bypass the node_modules/.bin folder when calling commands...

@illarionvk
Copy link
Author

@barneycarroll This is a standard NPM behavior:

Any bin files are symlinked to ./node_modules/.bin/, so that they may be found by npm scripts when necessary.
https://docs.npmjs.com/files/folders

@barneycarroll
Copy link
Member

I'm extremely surprised by this behaviour. Apparently it was introduced in npm@3 as a necessary measure to allow dependency scripts to function with a flattened dependency tree. But the notion that dependency scripts should affect your path really seems like an unacceptable oversight to that solution.

The change to Mithril you're proposing makes sense, but the underlying issue is an NPM bug AFAIC. What if a dependency has a cd script? Should all NPM packages ensure they have globally unique script keys? 🤔

@illarionvk
Copy link
Author

illarionvk commented Mar 1, 2017

Yarn doesn't symlink stuff to .bin folder and a lot of people think it breaks compatibility.

You have control over the symlink names of Mithril's binaries in package.json:

https://github.com/lhorie/mithril.js/blob/next/package.json#L32

@dead-claudia
Copy link
Member

:-\

And this adds to the list of reasons why I feel it shouldn't be publicly exported. (I've noted this opinion of mine multiple times in the past.)

@barneycarroll
Copy link
Member

OK I totally missed the point @illarionvk, thought you were talking about npm script references as opposed to bins. I don't see any reason why ospec and bundler can't be referenced by explicit path since they're only used in npm scripts anyway. What do we think?

@dead-claudia
Copy link
Member

@barneycarroll ospec is okay exporting a binary, since it's like Mocha and Tape, and is a more general purpose test utility. It shouldn't be published with the mithril package, as that's feature creep. bundler is very specific to Mithril, and is hardly useful anywhere else, as has been noted on several occasions by @tivac and I, so that should be relegated to explicit paths.

@lhorie
Copy link
Member

lhorie commented Mar 31, 2017

^ agree

@dead-claudia dead-claudia added this to the 2.0.0 milestone Jun 15, 2017
tivac added a commit to tivac/mithril.js that referenced this issue Jul 19, 2017
It's an internal-only thing, so publishing it doesn't make much sense.

Fixes MithrilJS#1668
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

5 participants