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

Improves the sdk #646

Merged
merged 3 commits into from
Dec 20, 2019
Merged

Improves the sdk #646

merged 3 commits into from
Dec 20, 2019

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Dec 20, 2019

What's the problem this PR addresses?

The current SDK files had to run within a part of the dependency tree in order to be able to access the original files (for example, the ESLint SDK was doing a require('eslint')). This started to cause issues with the new vscode-eslint version, which uses NODE_PATH in order to detect the path of the ESLint API.

How did you fix it?

  • The PnP loader is told to use the native resolution when resolving from locations inside the SDK files (this means that calling require('eslint', {paths:['.vscode/pnpify']}) will now go into the classic resolution and thus properly support NODE_PATH).

  • The SDK files now get the path of the modules they need to resolve by running a resolution on behalf of the project root (rather than themselves, since they're now considered as being outside of the dependency tree, cf previous point).

  • The PnP ignore mechanism has been revamped and now accepts a list of glob patterns instead of a single regex. The paths are compared relative to the project root instead of being checked against the absolute path of the issuer.

  • Even if the issuer is covered by an ignore rule, the request will still go to the PnP resolver if it can be proved to belong to a package inside the dependency tree (which only happens if the requested module is an absolute path). This is because the native resolver otherwise cannot load files from within zip archives (the native resolver doesn't use the fs extensions, and instead uses its own native binding of stat).

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.

1 participant