Skip to content

Commit 57c8a78

Browse files
evanwevanw
authored
Update Yarn PnP to match the latest specification (#2453)
1 parent c223771 commit 57c8a78

File tree

5 files changed

+158
-36
lines changed

5 files changed

+158
-36
lines changed

‎internal/fs/fs_zip.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ func (fs *zipFS) Abs(path string) (string, bool) {
279279
}
280280

281281
func (fs *zipFS) Dir(path string) string {
282-
if prefix, suffix, ok := parseYarnPnPVirtualPath(path); ok && suffix == "" {
282+
if prefix, suffix, ok := ParseYarnPnPVirtualPath(path); ok && suffix == "" {
283283
return prefix
284284
}
285285
return fs.inner.Dir(path)
@@ -313,7 +313,7 @@ func (fs *zipFS) WatchData() WatchData {
313313
return fs.inner.WatchData()
314314
}
315315

316-
func parseYarnPnPVirtualPath(path string) (string, string, bool) {
316+
func ParseYarnPnPVirtualPath(path string) (string, string, bool) {
317317
i := 0
318318

319319
for {
@@ -324,8 +324,13 @@ func parseYarnPnPVirtualPath(path string) (string, string, bool) {
324324
}
325325
i += slash + 1
326326

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

374379
func mangleYarnPnPVirtualPath(path string) string {
375-
if prefix, suffix, ok := parseYarnPnPVirtualPath(path); ok {
380+
if prefix, suffix, ok := ParseYarnPnPVirtualPath(path); ok {
376381
return prefix + suffix
377382
}
378383
return path

‎internal/resolver/resolver.go

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,21 +1189,45 @@ func (r resolverQuery) dirInfoUncached(path string) *dirInfo {
11891189
}
11901190
}
11911191

1192-
// Record if this directory has a Yarn PnP data file
1193-
if pnp, _ := entries.Get(".pnp.data.json"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
1194-
absPath := r.fs.Join(path, ".pnp.data.json")
1195-
if json := r.extractYarnPnPDataFromJSON(absPath, &r.caches.JSONCache); json.Data != nil {
1196-
info.pnpData = compileYarnPnPData(absPath, path, json)
1197-
}
1198-
} else if pnp, _ := entries.Get(".pnp.cjs"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
1199-
absPath := r.fs.Join(path, ".pnp.cjs")
1200-
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
1201-
info.pnpData = compileYarnPnPData(absPath, path, json)
1202-
}
1203-
} else if pnp, _ := entries.Get(".pnp.js"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
1204-
absPath := r.fs.Join(path, ".pnp.js")
1205-
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
1206-
info.pnpData = compileYarnPnPData(absPath, path, json)
1192+
// Record if this directory has a Yarn PnP manifest. This must not be done
1193+
// for Yarn virtual paths because that will result in duplicate copies of
1194+
// the same manifest which will result in multiple copies of the same virtual
1195+
// directory in the same path, which we don't handle (and which also doesn't
1196+
// match Yarn's behavior).
1197+
//
1198+
// For example, imagine a project with a manifest here:
1199+
//
1200+
// /project/.pnp.cjs
1201+
//
1202+
// and a source file with an import of "bar" here:
1203+
//
1204+
// /project/.yarn/__virtual__/pkg/1/foo.js
1205+
//
1206+
// If we didn't ignore Yarn PnP manifests in virtual folders, then we would
1207+
// pick up on the one here:
1208+
//
1209+
// /project/.yarn/__virtual__/pkg/1/.pnp.cjs
1210+
//
1211+
// which means we would potentially resolve the import to something like this:
1212+
//
1213+
// /project/.yarn/__virtual__/pkg/1/.yarn/__virtual__/pkg/1/bar
1214+
//
1215+
if _, _, ok := fs.ParseYarnPnPVirtualPath(path); !ok {
1216+
if pnp, _ := entries.Get(".pnp.data.json"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
1217+
absPath := r.fs.Join(path, ".pnp.data.json")
1218+
if json := r.extractYarnPnPDataFromJSON(absPath, &r.caches.JSONCache); json.Data != nil {
1219+
info.pnpData = compileYarnPnPData(absPath, path, json)
1220+
}
1221+
} else if pnp, _ := entries.Get(".pnp.cjs"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
1222+
absPath := r.fs.Join(path, ".pnp.cjs")
1223+
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
1224+
info.pnpData = compileYarnPnPData(absPath, path, json)
1225+
}
1226+
} else if pnp, _ := entries.Get(".pnp.js"); pnp != nil && pnp.Kind(r.fs) == fs.FileEntry {
1227+
absPath := r.fs.Join(path, ".pnp.js")
1228+
if json := r.tryToExtractYarnPnPDataFromJS(absPath, &r.caches.JSONCache); json.Data != nil {
1229+
info.pnpData = compileYarnPnPData(absPath, path, json)
1230+
}
12071231
}
12081232
}
12091233

‎internal/resolver/testExpectations.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
[null, [
1414
[null, {
1515
"packageLocation": "./",
16-
"packageDependencies": [],
16+
"packageDependencies": [["test", "npm:1.0.0"]],
1717
"linkType": "SOFT"
1818
}]
1919
]],

‎internal/resolver/yarnpnp.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ func (r resolverQuery) pnpResolve(specifier string, parentURL string, parentMani
9191
return specifier, true
9292
}
9393

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

267-
// Return dependencyPkg.packageLocation concatenated with modulePath
268-
resolved := dependencyPkg.packageLocation + modulePath
269-
result := r.fs.Join(manifest.absDirPath, resolved)
270-
if strings.HasSuffix(resolved, "/") && !strings.HasSuffix(result, "/") {
267+
// Return path.resolve(manifest.dirPath, dependencyPkg.packageLocation, modulePath)
268+
result := r.fs.Join(manifest.absDirPath, dependencyPkg.packageLocation, modulePath)
269+
if !strings.HasSuffix(result, "/") && ((modulePath != "" && strings.HasSuffix(modulePath, "/")) ||
270+
(modulePath == "" && strings.HasSuffix(dependencyPkg.packageLocation, "/"))) {
271271
result += "/" // This is important for matching Yarn PnP's expectations in tests
272272
}
273273
if r.debugLogs != nil {

‎scripts/js-api-tests.js

Lines changed: 102 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2653,17 +2653,25 @@ require("/assets/file.png");
26532653
import foo from './test.zip/foo.js'
26542654
import bar from './test.zip/bar/bar.js'
26552655
2656-
import virtual1 from './test.zip/__virtual__/ignored/0/foo.js'
2657-
import virtual2 from './test.zip/ignored/__virtual__/ignored/1/foo.js'
2658-
import virtual3 from './test.zip/__virtual__/ignored/1/test.zip/foo.js'
2656+
import __virtual__1 from './test.zip/__virtual__/ignored/0/foo.js'
2657+
import __virtual__2 from './test.zip/ignored/__virtual__/ignored/1/foo.js'
2658+
import __virtual__3 from './test.zip/__virtual__/ignored/1/test.zip/foo.js'
2659+
2660+
import $$virtual1 from './test.zip/$$virtual/ignored/0/foo.js'
2661+
import $$virtual2 from './test.zip/ignored/$$virtual/ignored/1/foo.js'
2662+
import $$virtual3 from './test.zip/$$virtual/ignored/1/test.zip/foo.js'
26592663
26602664
console.log({
26612665
foo,
26622666
bar,
26632667
2664-
virtual1,
2665-
virtual2,
2666-
virtual3,
2668+
__virtual__1,
2669+
__virtual__2,
2670+
__virtual__3,
2671+
2672+
$$virtual1,
2673+
$$virtual2,
2674+
$$virtual3,
26672675
})
26682676
`)
26692677

@@ -2702,13 +2710,25 @@ require("/assets/file.png");
27022710
// scripts/.js-api-tests/zipFile/test.zip/__virtual__/ignored/1/test.zip/foo.js
27032711
var foo_default4 = "foo";
27042712
2713+
// scripts/.js-api-tests/zipFile/test.zip/$$virtual/ignored/0/foo.js
2714+
var foo_default5 = "foo";
2715+
2716+
// scripts/.js-api-tests/zipFile/test.zip/ignored/$$virtual/ignored/1/foo.js
2717+
var foo_default6 = "foo";
2718+
2719+
// scripts/.js-api-tests/zipFile/test.zip/$$virtual/ignored/1/test.zip/foo.js
2720+
var foo_default7 = "foo";
2721+
27052722
// scripts/.js-api-tests/zipFile/entry.js
27062723
console.log({
27072724
foo: foo_default,
27082725
bar: bar_default,
2709-
virtual1: foo_default2,
2710-
virtual2: foo_default3,
2711-
virtual3: foo_default4
2726+
__virtual__1: foo_default2,
2727+
__virtual__2: foo_default3,
2728+
__virtual__3: foo_default4,
2729+
$$virtual1: foo_default5,
2730+
$$virtual2: foo_default6,
2731+
$$virtual3: foo_default7
27122732
});
27132733
})();
27142734
`)
@@ -2965,6 +2985,79 @@ require("/assets/file.png");
29652985
// scripts/.js-api-tests/yarnPnP_pnp_cjs_JSON_parse_identifier/entry.js
29662986
console.log(left_pad_default());
29672987
})();
2988+
`)
2989+
},
2990+
2991+
async yarnPnP_ignoreNestedManifests({ esbuild, testDir }) {
2992+
const entry = path.join(testDir, 'entry.js')
2993+
const foo = path.join(testDir, 'foo', 'index.js')
2994+
const bar = path.join(testDir, 'bar', 'index.js')
2995+
const manifest = path.join(testDir, '.pnp.data.json')
2996+
2997+
await writeFileAsync(entry, `
2998+
import foo from 'foo'
2999+
console.log(foo)
3000+
`)
3001+
3002+
await mkdirAsync(path.dirname(foo), { recursive: true })
3003+
await writeFileAsync(foo, `
3004+
import bar from 'bar'
3005+
export default 'foo' + bar
3006+
`)
3007+
3008+
await mkdirAsync(path.dirname(bar), { recursive: true })
3009+
await writeFileAsync(bar, `
3010+
export default 'bar'
3011+
`)
3012+
3013+
await writeFileAsync(manifest, `{
3014+
"packageRegistryData": [
3015+
[null, [
3016+
[null, {
3017+
"packageLocation": "./",
3018+
"packageDependencies": [
3019+
["foo", "npm:1.0.0"],
3020+
["bar", "npm:1.0.0"]
3021+
],
3022+
"linkType": "SOFT"
3023+
}]
3024+
]],
3025+
["foo", [
3026+
["npm:1.0.0", {
3027+
"packageLocation": "./__virtual__/whatever/0/foo/",
3028+
"packageDependencies": [
3029+
["bar", "npm:1.0.0"]
3030+
],
3031+
"linkType": "HARD"
3032+
}]
3033+
]],
3034+
["bar", [
3035+
["npm:1.0.0", {
3036+
"packageLocation": "./__virtual__/whatever/0/bar/",
3037+
"packageDependencies": [],
3038+
"linkType": "HARD"
3039+
}]
3040+
]]
3041+
]
3042+
}`)
3043+
3044+
const value = await esbuild.build({
3045+
entryPoints: [entry],
3046+
bundle: true,
3047+
write: false,
3048+
})
3049+
3050+
assert.strictEqual(value.outputFiles.length, 1)
3051+
assert.strictEqual(value.outputFiles[0].text, `(() => {
3052+
// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/__virtual__/whatever/0/bar/index.js
3053+
var bar_default = "bar";
3054+
3055+
// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/__virtual__/whatever/0/foo/index.js
3056+
var foo_default = "foo" + bar_default;
3057+
3058+
// scripts/.js-api-tests/yarnPnP_ignoreNestedManifests/entry.js
3059+
console.log(foo_default);
3060+
})();
29683061
`)
29693062
},
29703063
}

0 commit comments

Comments
 (0)