Skip to content

Commit

Permalink
fix #2495: strip non-go ignorePatternData stuff
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 10, 2022
1 parent 8fc9476 commit bde12f1
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 3 deletions.
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,15 @@

So instead of publishing a native esbuild binary executable to npm, this release publishes a WebAssembly fallback build. This is essentially the same as the `esbuild-wasm` package but it's installed automatically when you install the `esbuild` package on Android ARM. So packages that depend on the `esbuild` package should now work on Android ARM. This change has not yet been tested end-to-end because I don't have a 32-bit Android ARM device myself, but in theory it should work.

This inherits the drawbacks of WebAssembly including significantly slower performance than native as well as potentially also more severe memory usage limitations. If you want to use a native binary executable of esbuild on Android ARM, you may be able to build it yourself from source after installing the Android build tools.
This inherits the drawbacks of WebAssembly including significantly slower performance than native as well as potentially also more severe memory usage limitations and lack of certain features (e.g. `--serve`). If you want to use a native binary executable of esbuild on Android ARM, you may be able to build it yourself from source after installing the Android build tools.

* Attempt to better support Yarn's `ignorePatternData` feature ([#2495](https://github.com/evanw/esbuild/issues/2495))

Part of resolving paths in a project using Yarn's Plug'n'Play feature involves evaluating a regular expression in the `ignorePatternData` property of `.pnp.data.json`. However, it turns out that the particular regular expressions generated by Yarn use some syntax that works with JavaScript regular expressions but that does not work with Go regular expressions.

In this release, esbuild will now strip some of the the problematic syntax from the regular expression before compiling it, which should hopefully allow it to be compiled by Go's regular expression engine. The two character sequences that esbuild currently strips are `(?!\.{1,2}(?:\/|$))` and `(?!(?:^|\/)\.{1,2}(?:\/|$))` which seem to be used by Yarn to avoid the `.` and `..` path segments in the middle of relative paths. The removal of these character sequences seems relatively harmless in this case since esbuild shouldn't ever generate such path segments.

This change should add support to esbuild for Yarn's [`pnpIgnorePatterns`](https://yarnpkg.com/configuration/yarnrc/#pnpIgnorePatterns) feature.

## 0.15.7

Expand Down
3 changes: 3 additions & 0 deletions internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,9 @@ func (rr *resolver) Resolve(sourceDir string, importPath string, kind ast.Import
r.pnpManifest = compileYarnPnPData(absPath, r.fs.Dir(absPath), json)
}
}
if r.debugLogs != nil && r.pnpManifest != nil && r.pnpManifest.invalidIgnorePatternData != "" {
r.debugLogs.addNote(" Invalid Go regular expression for \"ignorePatternData\": " + r.pnpManifest.invalidIgnorePatternData)
}
break
}
}
Expand Down
22 changes: 20 additions & 2 deletions internal/resolver/yarnpnp.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ type pnpData struct {
// the classic Node.js resolution algorithm rather than the Plug'n'Play one.
// Note that unlike other paths in the manifest, the one checked against this
// regexp won't begin by `./`.
ignorePatternData *regexp.Regexp
ignorePatternData *regexp.Regexp
invalidIgnorePatternData string

// This is the main part of the PnP data file. This table contains the list
// of all packages, first keyed by package ident then by package reference.
Expand Down Expand Up @@ -438,7 +439,24 @@ func compileYarnPnPData(absPath string, absDirPath string, json js_ast.Expr) *pn

if value, _, ok := getProperty(json, "ignorePatternData"); ok {
if ignorePatternData, ok := getString(value); ok {
data.ignorePatternData, _ = regexp.Compile(ignorePatternData)
// The Go regular expression engine doesn't support some of the features
// that JavaScript regular expressions support, including "(?!" negative
// lookaheads which Yarn uses. This is deliberate on Go's part. See this:
// https://github.com/golang/go/issues/18868.
//
// Yarn uses this feature to exclude the "." and ".." path segments in
// the middle of a relative path. However, we shouldn't ever generate
// such path segments in the first place. So as a hack, we just remove
// the specific character sequences used by Yarn for this so that the
// regular expression is more likely to be able to be compiled.
ignorePatternData = strings.ReplaceAll(ignorePatternData, `(?!\.{1,2}(?:\/|$))`, "")
ignorePatternData = strings.ReplaceAll(ignorePatternData, `(?!(?:^|\/)\.{1,2}(?:\/|$))`, "")

if reg, err := regexp.Compile(ignorePatternData); err == nil {
data.ignorePatternData = reg
} else {
data.invalidIgnorePatternData = ignorePatternData
}
}
}

Expand Down
1 change: 1 addition & 0 deletions require/yarnpnp/bar/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.bar = require('bar-pkg').bar
1 change: 1 addition & 0 deletions require/yarnpnp/bar/node_modules/bar-pkg/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions require/yarnpnp/in.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@ if (mm.default.getType('txt') !== 'text/plain') throw '❌ mime'
import * as foo from 'foo'
if (foo.default !== 'foo') throw '❌ foo'

import * as bar from './bar/index.js'
if (bar.bar !== 'bar') throw '❌ bar'

console.log('✅ Yarn PnP tests passed')
3 changes: 3 additions & 0 deletions scripts/test-yarnpnp.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ function reinstallYarnIfNeeded() {
run('yarn set version 3.2.3')
}

const rc = fs.readFileSync(path.join(rootDir, '.yarnrc.yml'), 'utf8')
fs.writeFileSync(path.join(rootDir, '.yarnrc.yml'), `pnpIgnorePatterns: ["./bar/**"]\n` + rc)

run('yarn install')
}

Expand Down

0 comments on commit bde12f1

Please sign in to comment.