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.

Sign up for GitHub

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

Update Yarn PnP to match the latest specification #2453

Merged
merged 3 commits into from
Aug 10, 2022
Merged

Update Yarn PnP to match the latest specification #2453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c77053
yarn pnp: `$$virtual` is the same as `__virtual__`
evanw Aug 10, 2022
b7908b6
yarn pnp: handle other specification changes
evanw Aug 10, 2022
b9cae7d
yarn pnp: ignore manifests in __virtual__ folders
evanw Aug 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions internal/fs/fs_zip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func (fs *zipFS) Abs(path string) (string, bool) {
}

func (fs *zipFS) Dir(path string) string {
if prefix, suffix, ok := parseYarnPnPVirtualPath(path); ok && suffix == "" {
if prefix, suffix, ok := ParseYarnPnPVirtualPath(path); ok && suffix == "" {
return prefix
}
return fs.inner.Dir(path)
Expand Down Expand Up @@ -313,7 +313,7 @@ func (fs *zipFS) WatchData() WatchData {
return fs.inner.WatchData()
}

func parseYarnPnPVirtualPath(path string) (string, string, bool) {
func ParseYarnPnPVirtualPath(path string) (string, string, bool) {
i := 0

for {
Expand All @@ -324,8 +324,13 @@ func parseYarnPnPVirtualPath(path string) (string, string, bool) {
}
i += slash + 1

// Replace the segments "__virtual__/<segment>/<n>" with N times the ".." operation
if path[start:i-1] == "__virtual__" {
// Replace the segments "__virtual__/<segment>/<n>" with N times the ".."
// operation. Note: The "__virtual__" folder name appeared with Yarn 3.0.
// Earlier releases used "$$virtual", but it was changed after discovering
// that this pattern triggered bugs in software where paths were used as
// either regexps or replacement. For example, "$$" found in the second
// parameter of "String.prototype.replace" silently turned into "$".
if segment := path[start : i-1]; segment == "__virtual__" || segment == "$$virtual" {
if slash := strings.IndexAny(path[i:], "/\\"); slash != -1 {
var count string
var suffix string
Expand Down Expand Up @@ -372,7 +377,7 @@ func parseYarnPnPVirtualPath(path string) (string, string, bool) {
}

func mangleYarnPnPVirtualPath(path string) string {
if prefix, suffix, ok := parseYarnPnPVirtualPath(path); ok {
if prefix, suffix, ok := ParseYarnPnPVirtualPath(path); ok {
return prefix + suffix
}
return path
Expand Down
54 changes: 39 additions & 15 deletions internal/resolver/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,21 +1189,45 @@ func (r resolverQuery) dirInfoUncached(path string) *dirInfo {
}
}

// Record if this directory has a Yarn PnP data file
if pnp, _ := entries.Get(".pnp.data.json"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.data.json")
if json := r.extractYarnPnPDataFromJSON(absPath, &r.caches.JSONCache); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
}
} else if pnp, _ := entries.Get(".pnp.cjs"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.cjs")
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
}
} else if pnp, _ := entries.Get(".pnp.js"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.js")
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
// Record if this directory has a Yarn PnP manifest. This must not be done
// for Yarn virtual paths because that will result in duplicate copies of
// the same manifest which will result in multiple copies of the same virtual
// directory in the same path, which we don't handle (and which also doesn't
// match Yarn's behavior).
//
// For example, imagine a project with a manifest here:
//
// /project/.pnp.cjs
//
// and a source file with an import of "bar" here:
//
// /project/.yarn/__virtual__/pkg/1/foo.js
//
// If we didn't ignore Yarn PnP manifests in virtual folders, then we would
// pick up on the one here:
//
// /project/.yarn/__virtual__/pkg/1/.pnp.cjs
//
// which means we would potentially resolve the import to something like this:
//
// /project/.yarn/__virtual__/pkg/1/.yarn/__virtual__/pkg/1/bar
//
if _, _, ok := fs.ParseYarnPnPVirtualPath(path); !ok {
if pnp, _ := entries.Get(".pnp.data.json"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.data.json")
if json := r.extractYarnPnPDataFromJSON(absPath, &r.caches.JSONCache); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
}
} else if pnp, _ := entries.Get(".pnp.cjs"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.cjs")
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
}
} else if pnp, _ := entries.Get(".pnp.js"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
absPath := r.fs.Join(path, ".pnp.js")
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
info.pnpData = compileYarnPnPData(absPath, path, json)
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/resolver/testExpectations.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
[null, [
[null, {
"packageLocation": "./",
"packageDependencies": [],
"packageDependencies": [["test", "npm:1.0.0"]],
"linkType": "SOFT"
}]
]],
Expand Down
12 changes: 6 additions & 6 deletions internal/resolver/yarnpnp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ func (r resolverQuery) pnpResolve(specifier string, parentURL string, parentMani
return specifier, true
}

// Otherwise, if specifier starts with "/", "./", or "../", then
if strings.HasPrefix(specifier, "/") || strings.HasPrefix(specifier, "./") || strings.HasPrefix(specifier, "../") {
// Otherwise, if `specifier` is either an absolute path or a path prefixed with "./" or "../", then
if r.fs.IsAbs(specifier) || strings.HasPrefix(specifier, "./") || strings.HasPrefix(specifier, "../") {
// Set resolved to NM_RESOLVE(specifier, parentURL) and return it
return specifier, true
}
Expand Down Expand Up @@ -264,10 +264,10 @@ func (r resolverQuery) resolveToUnqualified(specifier string, parentURL string,
r.debugLogs.addNote(fmt.Sprintf(" Found package %q at %q", ident, dependencyPkg.packageLocation))
}

// Return dependencyPkg.packageLocation concatenated with modulePath
resolved := dependencyPkg.packageLocation + modulePath
result := r.fs.Join(manifest.absDirPath, resolved)
if strings.HasSuffix(resolved, "/") && !strings.HasSuffix(result, "/") {
// Return path.resolve(manifest.dirPath, dependencyPkg.packageLocation, modulePath)
result := r.fs.Join(manifest.absDirPath, dependencyPkg.packageLocation, modulePath)
if !strings.HasSuffix(result, "/") && ((modulePath != "" && strings.HasSuffix(modulePath, "/")) ||
(modulePath == "" && strings.HasSuffix(dependencyPkg.packageLocation, "/"))) {
result += "/" // This is important for matching Yarn PnP's expectations in tests
}
if r.debugLogs != nil {
Expand Down
111 changes: 102 additions & 9 deletions scripts/js-api-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2653,17 +2653,25 @@ require("/assets/file.png");
import foo from './test.zip/foo.js'
import bar from './test.zip/bar/bar.js'

import virtual1 from './test.zip/__virtual__/ignored/0/foo.js'
import virtual2 from './test.zip/ignored/__virtual__/ignored/1/foo.js'
import virtual3 from './test.zip/__virtual__/ignored/1/test.zip/foo.js'
import __virtual__1 from './test.zip/__virtual__/ignored/0/foo.js'
import __virtual__2 from './test.zip/ignored/__virtual__/ignored/1/foo.js'
import __virtual__3 from './test.zip/__virtual__/ignored/1/test.zip/foo.js'

import $$virtual1 from './test.zip/$$virtual/ignored/0/foo.js'
import $$virtual2 from './test.zip/ignored/$$virtual/ignored/1/foo.js'
import $$virtual3 from './test.zip/$$virtual/ignored/1/test.zip/foo.js'

console.log({
foo,
bar,

virtual1,
virtual2,
virtual3,
__virtual__1,
__virtual__2,
__virtual__3,

$$virtual1,
$$virtual2,
$$virtual3,
})
`)

Expand Down Expand Up @@ -2702,13 +2710,25 @@ require("/assets/file.png");
// scripts/.js-api-tests/zipFile/test.zip/__virtual__/ignored/1/test.zip/foo.js
var foo_default4 = "foo";

// scripts/.js-api-tests/zipFile/test.zip/$$virtual/ignored/0/foo.js
var foo_default5 = "foo";

// scripts/.js-api-tests/zipFile/test.zip/ignored/$$virtual/ignored/1/foo.js
var foo_default6 = "foo";

// scripts/.js-api-tests/zipFile/test.zip/$$virtual/ignored/1/test.zip/foo.js
var foo_default7 = "foo";

// scripts/.js-api-tests/zipFile/entry.js
console.log({
foo: foo_default,
bar: bar_default,
virtual1: foo_default2,
virtual2: foo_default3,
virtual3: foo_default4
__virtual__1: foo_default2,
__virtual__2: foo_default3,
__virtual__3: foo_default4,
$$virtual1: foo_default5,
$$virtual2: foo_default6,
$$virtual3: foo_default7
});
})();
`)
Expand Down Expand Up @@ -2965,6 +2985,79 @@ require("/assets/file.png");
// scripts/.js-api-tests/yarnPnP_pnp_cjs_JSON_parse_identifier/entry.js
console.log(left_pad_default());
})();
`)
},

async yarnPnP_ignoreNestedManifests({ esbuild, testDir }) {
const entry = path.join(testDir, 'entry.js')
const foo = path.join(testDir, 'foo', 'index.js')
const bar = path.join(testDir, 'bar', 'index.js')
const manifest = path.join(testDir, '.pnp.data.json')

await writeFileAsync(entry, `
import foo from 'foo'
console.log(foo)
`)

await mkdirAsync(path.dirname(foo), { recursive: true })
await writeFileAsync(foo, `
import bar from 'bar'
export default 'foo' + bar
`)

await mkdirAsync(path.dirname(bar), { recursive: true })
await writeFileAsync(bar, `
export default 'bar'
`)

await writeFileAsync(manifest, `{
"packageRegistryData": [
[null, [
[null, {
"packageLocation": "./",
"packageDependencies": [
["foo", "npm:1.0.0"],
["bar", "npm:1.0.0"]
],
"linkType": "SOFT"
}]
]],
["foo", [
["npm:1.0.0", {
"packageLocation": "./__virtual__/whatever/0/foo/",
"packageDependencies": [
["bar", "npm:1.0.0"]
],
"linkType": "HARD"
}]
]],
["bar", [
["npm:1.0.0", {
"packageLocation": "./__virtual__/whatever/0/bar/",
"packageDependencies": [],
"linkType": "HARD"
}]
]]
]
}`)

const value = await esbuild.build({
entryPoints: [entry],
bundle: true,
write: false,
})

assert.strictEqual(value.outputFiles.length, 1)
assert.strictEqual(value.outputFiles[0].text, `(() => {
// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/__virtual__/whatever/0/bar/index.js
var bar_default = "bar";

// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/__virtual__/whatever/0/foo/index.js
var foo_default = "foo" + bar_default;

// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/entry.js
console.log(foo_default);
})();
`)
},
}
Expand Down