can't fork and use this npm module via git in package.json

[jeffreywescott]

I just submitted a pull request (see #51) to fix the fact that this library strips window.location.hash from the url (which breaks in-page offset navigation when using react-router. Ordinarily, when I find a problem with a library and make a fix, I can just point to my github version of the library and the branch name in my package.json and I'm good to go. But because of the strange build system this library uses, this is not possible, forcing everyone to wait for a new release from the primary author.

Unless I'm missing something (which is entirely possible) ...

[mjackson]

This has honestly been really frustrating for me too. Let me explain how things are currently setup, and hopefully we can figure out a better way to do things.

Our build isn't really that strange–it's just a single babel transpile step, which is quite common nowadays, especially in the React community. We use npm's prepublish script to create the build, which AFAICT is in accordance with npm's script guidelines for running a build step that transpiles one language to another (e.g. CoffeeScript).

The problem in this case is that, although npm runs prepublish on a local npm install, it won't do that for remotes (like GitHub). So, yes. Installing from GitHub is broken.

Perhaps @othiym23 has an idea how we can improve this...

[mjackson]

I guess we could use a postinstall hook that generates the lib directory if there isn't one already. That might work.

[mjackson]

Either that or we could check our lib directory into git. But I'd prefer to avoid that.

[gaearon]

Note even with postinstall it's a bit tricky. See how Radium does it.

cc @ianobermiller who knows why it's done this way.

[mjackson]

@gaearon Ya, that's exactly what I was thinking.

Basically, if the lib directory is missing then we know we were not installed from the npm package. So build it.

[jeffreywescott]

Wow. Thanks for the quick turnaround on this, guys. Not sure if that commit fixed it (I haven't tried it), but I'll give it a try and let you know.

[mjackson]

@jeffreywescott Nah, that commit doesn't actually fix the problem because the modules directory is in our .npmignore. npm currently doesn't handle the transpilation use case very well at all it seems :/

In short, we could fix remote installs but it would mean that we now have to include the ES2015 source (the modules dir, and probably our .babelrc too) in our npm package. I dunno. Maybe that's something we actually want to do. Use lib for our build, modules for the source. There are a few niche tools that actually support running ES2015 nowadays, and that crowd is only going to get bigger.

[gaearon]

it would mean that we now have to include the ES2015 source (the modules dir, and probably our .babelrc too) in our npm package

I started doing this, and I think it's the way to go. Some people want to look into uncompiled code, or even use ESnext env. And it handles this use case too. Go for it!

[jeffreywescott]

Hey, guys.

I just tried this again today, and things are still not working. I thought the reason why was that package.tgz does not include the .babelrc file that's necessary for building:

$ npm install
/
> [email protected] postinstall /Users/jeffrey/dev/wisdomlabs/wisdomlabs-com/node_modules/history
> node -e "require('fs').stat('lib', function (e, s) { process.exit(e || !s.isDirectory() ? 1 : 0) })" || npm run build


> [email protected] build /Users/jeffrey/dev/wisdomlabs/wisdomlabs-com/node_modules/history
> babel ./modules -d lib --ignore '__tests__'

modules/Actions.js -> lib/Actions.js
modules/AsyncUtils.js -> lib/AsyncUtils.js
modules/DOMStateStorage.js -> lib/DOMStateStorage.js
modules/DOMUtils.js -> lib/DOMUtils.js
modules/ExecutionEnvironment.js -> lib/ExecutionEnvironment.js
modules/createBrowserHistory.js -> lib/createBrowserHistory.js
modules/createDOMHistory.js -> lib/createDOMHistory.js
modules/createHashHistory.js -> lib/createHashHistory.js
modules/createHistory.js -> lib/createHistory.js
modules/createLocation.js -> lib/createLocation.js
modules/createMemoryHistory.js -> lib/createMemoryHistory.js
modules/enableBeforeUnload.js -> lib/enableBeforeUnload.js
modules/enableQueries.js -> lib/enableQueries.js
SyntaxError: modules/index.js: Unexpected token (1:7)
> 1 | export createHistory from './createBrowserHistory'
    |        ^
  2 | export createHashHistory from './createHashHistory'
  3 | export createMemoryHistory from './createMemoryHistory'
  4 | export createLocation from './createLocation'

npm ERR! Darwin 14.5.0
npm ERR! argv "/Users/jeffrey/.nvm/versions/io.js/v2.4.0/bin/iojs" "/Users/jeffrey/.nvm/versions/io.js/v2.4.0/bin/npm" "run" "build"
npm ERR! node v2.4.0
npm ERR! npm  v2.13.0
npm ERR! code ELIFECYCLE
npm ERR! [email protected] build: `babel ./modules -d lib --ignore '__tests__'`
npm ERR! Exit status 1

However, even adding --stage 0 --loose to the npm build script in package.json didn't seem to help.

Any ideas?

[mjackson]

AFAICT you need to use --loose all @jeffreywescott, not just --loose.

[mjackson]

I just confirmed that with --loose all it works, @jeffreywescott. I'll push an update to master.