search

browserify / browser-resolve

resolve function which support the browser field in package.json
MIT License
102 stars 70 forks source link

Resolve symlinked modules correctly #92

Closed lhorie closed 4 years ago

lhorie commented 5 years ago

Some systems (e.g. rush, pnpm) use symlinks to create a recursive dependency graph instead of relying on the hoisting aspect of the node module resolution algorithm (like npm and yarn do) in these cases, we want to resolve to the real path of a module, so that the tree structure below only ever tries to run the x module once (rather than once on each module that depends on it)

- node_modules
  - a
    - node_modules
      - symlink to x
  - b
    - node_modules
      - symlink to x

There's an example of a downstream bug here: https://github.com/lhorie/jest-browser-bug (via https://github.com/facebook/jest/issues/7840)

lhorie commented 5 years ago

CI now passes for Node 6.

Current CI failures happen on older versions of node on code I didn't touch (due to ES6 syntax).

lhorie commented 5 years ago

@defunctzombie Ping. Is this package still maintained?

lhorie commented 5 years ago

@defunctzombie I made the code style change and moved the test setup script to the test script

rtsao commented 4 years ago

@defunctzombie Is there anything else that needs to be done for this to be ready to merge?

I suppose get tests passing in older version of node?

defunctzombie commented 4 years ago

It's fair to say that I don't maintain or keep this module updated very much. Which upstream module(s) depend on this enough to have their maintainers consider picking up this module as well?

rtsao commented 4 years ago

I see. Jest is currently using this package (see https://github.com/facebook/jest/pull/9511). I suppose the Jest maintainers might be amenable to taking over ownership (or at the very least forking their own version to address their needs).

defunctzombie commented 4 years ago

@rtsao I'd be happy to see this PR land if the tests were updated to pass on the versions of node that jest actively supports - I don't care if old node version support is dropped. If you want to prepare another PR building from this one and ping me I can merge it.

I am also happy to have jest maintainers pickup this module or logic if that is easier for them. I do not commit resources to this module.

rtsao commented 4 years ago

Thanks @defunctzombie!

@lhorie was kind enough to grant me push access so I've just made the changes here.

As of Jest v25, Node 6.x support was removed. Additionally, Node 6.x EOL was April 30, 2019. So I've updated the travis.yml to test against 8.x, 10.x and 12.x and added an npm lockfile so that tests will be deterministic and won't break in the future.