Skip to content

Conversation

@keithamus
Copy link
Contributor

What?

This adds a call to require.resolve within dependency-graph, as require.resolve will canonicalize any symlinks within the path, to get the fully qualified path.

Why?

This allows us to use symlinks (such as npm link or yarn workspaces) and have the dependency graph resolve to the fully qualified files on disk. Without this change the dependency graph will only read the path, where the linter will lookup and resolve the existing files, and so there becomes a mismatch. Resolving symlinks resolves the mismatch.

We could optionally avoid using require.resolve and use fs.readlink - but fs.readlink will not canonicalize paths, in other words if node_modules/foo was a symlink then fs.readlink('node_modules/foo') would resolve, but fs.readlink('node_modules/foo/bar.js') would throw (other methods like fs.readFile('node_modules/foo/bar.js') will resolve as they do canonicalise symlinks).

@keithamus keithamus requested review from a team and josh October 17, 2019 10:20
Copy link
Contributor

@koddsson koddsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me but I'll leave the approval to someone else since we worked on this together.

@keithamus keithamus merged commit dc2f3e8 into master Oct 21, 2019
@keithamus keithamus deleted the feat-follow-symlinks-in-dependency-graph branch October 21, 2019 14:22
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

Successfully merging this pull request may close these issues.

4 participants