Skip to content

packageFilter paths should be realpathed with preserveSymlinks: false #201

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

Closed
bterlson opened this issue Nov 19, 2019 · 6 comments
Closed

Comments

@bterlson
Copy link

Right now a package filter is called with potentially symlinked paths.

@ljharb
Copy link
Member

ljharb commented Nov 19, 2019

Thanks! I'll take a look, but a PR with a test case would be super helpful.

@bterlson
Copy link
Author

@ljharb will try to put up a PR, but test cases are really annoying to make when symlinking is involved. Here's how to repro, though:

Directory structure like:

common
  node_modules
    mod-a
project
  node_modules
    mod-a -> ..\..\common\node_modules\mod-a

cd into project, open a node console, and enter:

> require('resolve').sync('mod-a', {
    preserveSymlinks: false,
    packageFilter (_, path) { console.log(path) }
  });
// console logs a symlinked path for the package json, but returns a realpath as expected.

@ljharb
Copy link
Member

ljharb commented Nov 20, 2019

@bterlson in your example, there's no package.jsons - packageFilter only kicks in when there is one. Do you also have a package.json inside mod-a?

@bterlson
Copy link
Author

@ljharb yessir, and main files, although that part is probably not necessary to highlight the problem.

@ljharb
Copy link
Member

ljharb commented Nov 21, 2019

Writing this failing test exposed two more unrelated bugs (#202 covers one of them) that I'll need to test and fix separately :-) This one's coming up shortly tho!

ljharb added a commit that referenced this issue Nov 21, 2019
ljharb added a commit that referenced this issue Nov 21, 2019
ljharb added a commit that referenced this issue Nov 22, 2019
…alpath the `pkgfile` option

 - `async`/`sync`: avoid crash when `package.json` does not exist, or `packageFilter` returns a falsy value

Fixes #201.
ljharb added a commit that referenced this issue Nov 22, 2019
…alpath the `pkgfile` option

 - `async`/`sync`: avoid crash when `package.json` does not exist, or `packageFilter` returns a falsy value

Fixes #201.
ljharb added a commit that referenced this issue Nov 22, 2019
…alpath the `pkgfile` option

 - `async`/`sync`: avoid crash when `package.json` does not exist, or `packageFilter` returns a falsy value

Fixes #201.
ljharb added a commit that referenced this issue Nov 22, 2019
…alpath the `pkgfile` option

 - `async`/`sync`: avoid crash when `package.json` does not exist, or `packageFilter` returns a falsy value

Fixes #201.
ljharb added a commit that referenced this issue Nov 22, 2019
…alpath the `pkgfile` option

 - `async`/`sync`: avoid crash when `package.json` does not exist, or `packageFilter` returns a falsy value

Fixes #201.
ljharb added a commit that referenced this issue Nov 22, 2019
 - [Fix] `sync`: `packageFilter`: when `preserveSymlinks` is `false`, realpath the `pkgfile` option (#201)
 - [Docs] fix CI status badge (#200)
 - [meta] add `funding` field
 - [Dev Deps] update `eslint`, `@ljharb/eslint-config`, `safe-publish-latest`
 - [Tests] use shared travis-ci configs
@ljharb
Copy link
Member

ljharb commented Nov 22, 2019

v1.12.1 v1.12.2 is released.

Note that this includes a bugfix where packageFilter was incorrectly getting called with the documented 2nd arg omitted; any sync packageFilter callbacks will likely need to be altered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants