Skip to content

Commit

Permalink
fix #1827: pass query/hash suffix to plugins
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Dec 12, 2021
1 parent 4af88a9 commit 049f99c
Show file tree
Hide file tree
Showing 9 changed files with 180 additions and 2 deletions.
30 changes: 30 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,36 @@

If a CommonJS file contains a `"use strict"` directive, it could potentially be unintentionally disabled by esbuild when using the "inject" feature when bundling is enabled. This is because the inject feature was inserting a call to the initializer for the injected file before the `"use strict"` directive. In JavaScript, directives do not apply if they come after a non-directive statement. This release fixes the problem by moving the `"use strict"` directive before the initializer for the injected file so it isn't accidentally disabled.

* Pass the ignored path query/hash suffix to `onLoad` plugins ([#1827](https://github.com/evanw/esbuild/issues/1827))

The built-in `onResolve` handler that comes with esbuild can strip the query/hash suffix off of a path during path resolution. For example, `url("fonts/icons.eot?#iefix")` can be resolved to the file `fonts/icons.eot`. For context, IE8 has a bug where it considers the font face URL to extend to the last `)` instead of the first `)`. In the example below, IE8 thinks the URL for the font is `Example.eot?#iefix') format('eot'), url('Example.ttf') format('truetype` so by adding `?#iefix`, IE8 thinks the URL has a path of `Example.eot` and a query string of `?#iefix') format('eot...` and can load the font file:

```css
@font-face {
font-family: 'Example';
src: url('Example.eot?#iefix') format('eot'), url('Example.ttf') format('truetype');
}
```

However, the suffix is not currently passed to esbuild and plugins may want to use this suffix for something. Previously plugins had to add their own `onResolve` handler if they wanted to use the query suffix. With this release, the suffix can now be returned by plugins from `onResolve` and is now passed to plugins in `onLoad`:

```js
let examplePlugin = {
name: 'example',
setup(build) {
build.onResolve({ filter: /.*/ }, args => {
return { path: args.path, suffix: '?#iefix' }
})

build.onLoad({ filter: /.*/ }, args => {
console.log({ path: args.path, suffix: args.suffix })
})
},
}
```

The suffix is deliberately not included in the path that's provided to plugins because most plugins won't know to handle this strange edge case and would likely break. Keeping the suffix out of the path means that plugins can opt-in to handling this edge case if they want to, and plugins that aren't aware of this edge case will likely still do something reasonable.

## 0.14.2

* Add `[ext]` placeholder for path templates ([#1799](https://github.com/evanw/esbuild/pull/1799))
Expand Down
4 changes: 4 additions & 0 deletions cmd/esbuild/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,9 @@ func (service *serviceType) convertPlugins(key int, jsPlugins interface{}) ([]ap
if value, ok := response["namespace"]; ok {
result.Namespace = value.(string)
}
if value, ok := response["suffix"]; ok {
result.Suffix = value.(string)
}
if value, ok := response["external"]; ok {
result.External = value.(bool)
}
Expand Down Expand Up @@ -766,6 +769,7 @@ func (service *serviceType) convertPlugins(key int, jsPlugins interface{}) ([]ap
"ids": ids,
"path": args.Path,
"namespace": args.Namespace,
"suffix": args.Suffix,
"pluginData": args.PluginData,
}).(map[string]interface{})

Expand Down
3 changes: 3 additions & 0 deletions lib/shared/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
let pluginName = getFlag(result, keys, 'pluginName', mustBeString);
let path = getFlag(result, keys, 'path', mustBeString);
let namespace = getFlag(result, keys, 'namespace', mustBeString);
let suffix = getFlag(result, keys, 'suffix', mustBeString);
let external = getFlag(result, keys, 'external', mustBeBoolean);
let sideEffects = getFlag(result, keys, 'sideEffects', mustBeBoolean);
let pluginData = getFlag(result, keys, 'pluginData', canBeAnything);
Expand All @@ -805,6 +806,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
if (pluginName != null) response.pluginName = pluginName;
if (path != null) response.path = path;
if (namespace != null) response.namespace = namespace;
if (suffix != null) response.suffix = suffix;
if (external != null) response.external = external;
if (sideEffects != null) response.sideEffects = sideEffects;
if (pluginData != null) response.pluginData = stash.store(pluginData);
Expand All @@ -829,6 +831,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
let result = await callback({
path: request.path,
namespace: request.namespace,
suffix: request.suffix,
pluginData: stash.load(request.pluginData),
});

Expand Down
2 changes: 2 additions & 0 deletions lib/shared/stdio_protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ export interface OnResolveResponse {
external?: boolean;
sideEffects?: boolean;
namespace?: string;
suffix?: string;
pluginData?: number;

watchFiles?: string[];
Expand All @@ -182,6 +183,7 @@ export interface OnLoadRequest {
ids: number[];
path: string;
namespace: string;
suffix: string;
pluginData: number;
}

Expand Down
2 changes: 2 additions & 0 deletions lib/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ export interface OnResolveResult {
external?: boolean;
sideEffects?: boolean;
namespace?: string;
suffix?: string;
pluginData?: any;

watchFiles?: string[];
Expand All @@ -341,6 +342,7 @@ export interface OnLoadOptions {
export interface OnLoadArgs {
path: string;
namespace: string;
suffix: string;
pluginData: any;
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ type OnResolveResult struct {
External bool
SideEffects SideEffects
Namespace string
Suffix string
PluginData interface{}

WatchFiles []string
Expand All @@ -500,6 +501,7 @@ type OnLoadOptions struct {
type OnLoadArgs struct {
Path string
Namespace string
Suffix string
PluginData interface{}
}

Expand Down
12 changes: 11 additions & 1 deletion pkg/api/api_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1489,12 +1489,21 @@ func (impl *pluginImpl) OnResolve(options OnResolveOptions, callback func(OnReso
result.AbsWatchFiles = impl.validatePathsArray(response.WatchFiles, "watch file")
result.AbsWatchDirs = impl.validatePathsArray(response.WatchDirs, "watch directory")

// Restrict the suffix to start with "?" or "#" for now to match esbuild's behavior
if err == nil && response.Suffix != "" && response.Suffix[0] != '?' && response.Suffix[0] != '#' {
err = fmt.Errorf("Invalid path suffix %q returned from plugin (must start with \"?\" or \"#\")", response.Suffix)
}

if err != nil {
result.ThrownError = err
return
}

result.Path = logger.Path{Text: response.Path, Namespace: response.Namespace}
result.Path = logger.Path{
Text: response.Path,
Namespace: response.Namespace,
IgnoredSuffix: response.Suffix,
}
result.External = response.External
result.IsSideEffectFree = response.SideEffects == SideEffectsFalse
result.PluginData = response.PluginData
Expand Down Expand Up @@ -1527,6 +1536,7 @@ func (impl *pluginImpl) OnLoad(options OnLoadOptions, callback func(OnLoadArgs)
Path: args.Path.Text,
Namespace: args.Path.Namespace,
PluginData: args.PluginData,
Suffix: args.Path.IgnoredSuffix,
})
result.PluginName = response.PluginName
result.AbsWatchFiles = impl.validatePathsArray(response.WatchFiles, "watch file")
Expand Down
125 changes: 124 additions & 1 deletion scripts/plugin-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2060,14 +2060,137 @@ let pluginTests = {
setup(build) {
esbuildFromBuild = build.esbuild
build.onResolve({ filter: /.*/ }, () => ({ path: 'foo', namespace: 'bar' }))
build.onLoad({ filter: /.*/ }, async () => ({ contents: '' }))
build.onLoad({ filter: /.*/ }, () => ({ contents: '' }))
},
}],
})
if (esbuildFromBuild !== esbuild) {
throw new Error('Unexpected value for the "esbuild" property')
}
},

async onResolveInvalidPathSuffix({ testDir, esbuild }) {
try {
await esbuild.build({
entryPoints: ['foo'],
logLevel: 'silent',
plugins: [{
name: 'plugin',
setup(build) {
build.onResolve({ filter: /.*/ }, () => ({ path: 'bar', suffix: '%what' }))
},
}],
})
throw new Error('Expected an error to be thrown')
} catch (e) {
assert.strictEqual(e.message, `Build failed with 1 error:
error: Invalid path suffix "%what" returned from plugin (must start with "?" or "#")`)
}
},

async onResolveWithInternalOnLoadAndQuerySuffix({ testDir, esbuild }) {
const entry = path.join(testDir, 'entry.js')
await writeFileAsync(entry, `console.log('entry')`)
const onResolveSet = new Set()
const onLoadSet = new Set()
await esbuild.build({
stdin: {
resolveDir: testDir,
contents: `
import "foo%a"
import "foo%b"
`,
},
bundle: true,
write: false,
plugins: [{
name: 'plugin',
setup(build) {
build.onResolve({ filter: /.*/ }, args => {
onResolveSet.add({ path: args.path, suffix: args.suffix })
if (args.path.startsWith('foo%')) {
return {
path: entry,
suffix: '?' + args.path.slice(args.path.indexOf('%') + 1),
}
}
})
build.onLoad({ filter: /.*/ }, args => {
onLoadSet.add({ path: args.path, suffix: args.suffix })
})
},
}],
})
const order = (a, b) => {
a = JSON.stringify(a)
b = JSON.stringify(b)
return (a > b) - (a < b)
}
const observed = JSON.stringify({
onResolve: [...onResolveSet].sort(order),
onLoad: [...onLoadSet].sort(order),
}, null, 2)
const expected = JSON.stringify({
onResolve: [
{ path: 'foo%a' },
{ path: 'foo%b' },
],
onLoad: [
{ path: path.join(testDir, 'entry.js'), suffix: '?a' },
{ path: path.join(testDir, 'entry.js'), suffix: '?b' },
],
}, null, 2)
if (observed !== expected) throw new Error(`Observed ${observed}, expected ${expected}`)
},

async onLoadWithInternalOnResolveAndQuerySuffix({ testDir, esbuild }) {
const entry = path.join(testDir, 'entry.js')
await writeFileAsync(entry, `console.log('entry')`)
const onResolveSet = new Set()
const onLoadSet = new Set()
await esbuild.build({
stdin: {
resolveDir: testDir,
contents: `
import "./entry?a"
import "./entry?b"
`,
},
bundle: true,
write: false,
plugins: [{
name: 'plugin',
setup(build) {
build.onResolve({ filter: /.*/ }, args => {
onResolveSet.add({ path: args.path, suffix: args.suffix })
})
build.onLoad({ filter: /.*/ }, args => {
onLoadSet.add({ path: args.path, suffix: args.suffix })
})
},
}],
})
const order = (a, b) => {
a = JSON.stringify(a)
b = JSON.stringify(b)
return (a > b) - (a < b)
}
const observed = JSON.stringify({
onResolve: [...onResolveSet].sort(order),
onLoad: [...onLoadSet].sort(order),
}, null, 2)
const expected = JSON.stringify({
onResolve: [
{ path: './entry?a' },
{ path: './entry?b' },
],
onLoad: [
{ path: path.join(testDir, 'entry.js'), suffix: '?a' },
{ path: path.join(testDir, 'entry.js'), suffix: '?b' },
],
}, null, 2)
if (observed !== expected) throw new Error(`Observed ${observed}, expected ${expected}`)
},
}

// These tests have to run synchronously
Expand Down
2 changes: 2 additions & 0 deletions scripts/ts-type-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,13 @@ const tests = {
path: '',
external: true,
namespace: '',
suffix: '',
}
})
build.onLoad({filter: /./, namespace: ''}, args => {
let path: string = args.path;
let namespace: string = args.namespace;
let suffix: string = args.suffix;
if (Math.random()) return
if (Math.random()) return {}
return {
Expand Down

0 comments on commit 049f99c

Please sign in to comment.