Skip to content

Commit

Permalink
Merge pull request #39 from surma/fix-module-workers
Browse files Browse the repository at this point in the history
  • Loading branch information
@surma
surma authored Feb 24, 2021
2 parents 13a15b5 + a43a387 commit b6cc855
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 54 deletions.
118 changes: 70 additions & 48 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const { join } = require("path");
const ejs = require("ejs");
const MagicString = require("magic-string");
const tippex = require("tippex");
const json5 = require("json5");

const defaultOpts = {
// A string containing the EJS template for the amd loader. If `undefined`,
Expand All @@ -38,14 +39,21 @@ const defaultOpts = {
};

// A regexp to find static `new Worker` invocations.
// Matches `new Worker(...file part...`
// File part matches one of:
// - '...'
// - "..."
// - `import.meta.url`
// - `new URL('...', import.meta.url)
// - `new URL("...", import.meta.url)
// Also matches optional options param.
const workerRegexp = /(new\s+Worker\()\s*(('.*?'|".*?")|import\.meta\.url|new\s+URL\(('.*?'|".*?"),\s*import\.meta\.url\))\s*(,(.+?))?(\))/gs;
// - new URL('...', import.meta.url)
// - new URL("...", import.meta.url)
const workerRegexpForTransform = /(new\s+Worker\()\s*(('.*?'|".*?")|import\.meta\.url|new\s+URL\(('.*?'|".*?"),\s*import\.meta\.url\))/gs;

// A regexp to find static `new Worker` invocations we've rewritten during the transform phase.
// Matches `new Worker(...file part..., ...options...`.
// File part matches one of:
// - new URL('...', module.uri)
// - new URL("...", module.uri)
const workerRegexpForOutput = /new\s+Worker\(new\s+URL\((?:'.*?'|".*?"),\s*module\.uri\)\s*(,([^)]+))/gs;

let longWarningAlreadyShown = false;

Expand All @@ -57,30 +65,14 @@ module.exports = function(opts = {}) {
const urlLoaderPrefix = opts.urlLoaderScheme + ":";

let workerFiles;
let isEsmOutput = false;
let isEsmOutput = () => { throw new Error("outputOptions hasn't been called yet") };
return {
name: "off-main-thread",

async buildStart(options) {
workerFiles = [];
},

outputOptions({ format }) {
if (format === "esm" || format === "es") {
if (!opts.silenceESMWorkerWarning) {
this.warn(
'Very few browsers support ES modules in Workers. If you want to your code to run in all browsers, set `output.format = "amd";`'
);
}
// In ESM, we never prepend a loader.
isEsmOutput = true;
} else if (format !== "amd") {
this.error(
`\`output.format\` must either be "amd" or "esm", got "${format}"`
);
}
},

async resolveId(id, importer) {
if (!id.startsWith(urlLoaderPrefix)) return;

Expand Down Expand Up @@ -110,20 +102,19 @@ module.exports = function(opts = {}) {
// and string contents so it's more reliable on JS syntax.
tippex.match(
code,
workerRegexp,
workerRegexpForTransform,
(
fullMatch,
partBeforeArgs,
workerSource,
directWorkerFile,
workerFile,
optionsStrWithComma = "",
optionsStr = "",
partAfterArgs
) => {
// We need to get this before the `await`, otherwise `lastIndex`
// will be already overridden.
const matchIndex = workerRegexp.lastIndex - fullMatch.length;
const workerParametersEndIndex = workerRegexpForTransform.lastIndex;
const matchIndex = workerParametersEndIndex - fullMatch.length;
const workerParametersStartIndex = matchIndex + partBeforeArgs.length;

let workerIdPromise;
if (workerSource === "import.meta.url") {
Expand All @@ -132,12 +123,13 @@ module.exports = function(opts = {}) {
} else {
// Otherwise it's a string literal either directly or in the `new URL(...)`.
if (directWorkerFile) {
const fullReplacement = `new Worker(new URL(${directWorkerFile}, import.meta.url)${optionsStrWithComma})`;
const fullMatchWithOpts = `${fullMatch}, …)`;
const fullReplacement = `new Worker(new URL(${directWorkerFile}, import.meta.url), …)`;

if (!longWarningAlreadyShown) {
this.warn(
`rollup-plugin-off-main-thread:
\`${fullMatch}\` suggests that the Worker should be relative to the document, not the script.
\`${fullMatchWithOpts}\` suggests that the Worker should be relative to the document, not the script.
In the bundler, we don't know what the final document's URL will be, and instead assume it's a URL relative to the current module.
This might lead to incorrect behaviour during runtime.
If you did mean to use a URL relative to the current module, please change your code to the following form:
Expand All @@ -148,7 +140,7 @@ This will become a hard error in the future.`,
longWarningAlreadyShown = true;
} else {
this.warn(
`rollup-plugin-off-main-thread: Treating \`${fullMatch}\` as \`${fullReplacement}\``,
`rollup-plugin-off-main-thread: Treating \`${fullMatchWithOpts}\` as \`${fullReplacement}\``,
matchIndex
);
}
Expand All @@ -169,21 +161,6 @@ This will become a hard error in the future.`,
workerIdPromise = this.resolve(workerFile, id).then(res => res.id);
}

const workerParametersStartIndex = matchIndex + partBeforeArgs.length;
const workerParametersEndIndex =
matchIndex + fullMatch.length - partAfterArgs.length;

// Parse the optional options object if provided.
optionsStr = optionsStr.trim();
if (optionsStr) {
let optionsObject = new Function(`return ${optionsStr};`)();
if (!isEsmOutput) {
delete optionsObject.type;
}
optionsStr = JSON.stringify(optionsObject);
optionsStr = optionsStr === "{}" ? "" : `, ${optionsStr}`;
}

// tippex.match accepts only sync callback, but we want to perform &
// wait for async job here, so we track those promises separately.
replacementPromises.push(
Expand All @@ -198,7 +175,7 @@ This will become a hard error in the future.`,
ms.overwrite(
workerParametersStartIndex,
workerParametersEndIndex,
`new URL(import.meta.ROLLUP_FILE_URL_${chunkRefId}, import.meta.url)${optionsStr}`
`new URL(import.meta.ROLLUP_FILE_URL_${chunkRefId}, import.meta.url)`
);
})()
);
Expand All @@ -223,8 +200,26 @@ This will become a hard error in the future.`,
return JSON.stringify(chunk.relativePath);
},

outputOptions({ format }) {
if (format === "esm" || format === "es") {
if (!opts.silenceESMWorkerWarning) {
this.warn(
'Very few browsers support ES modules in Workers. If you want to your code to run in all browsers, set `output.format = "amd";`'
);
}
// In ESM, we never prepend a loader.
isEsmOutput = () => true;
} else if (format !== "amd") {
this.error(
`\`output.format\` must either be "amd" or "esm", got "${format}"`
);
} else {
isEsmOutput = () => false;
}
},

renderDynamicImport() {
if (isEsmOutput) return;
if (isEsmOutput()) return;

// In our loader, `require` simply return a promise directly.
// This is tinier and simpler output than the Rollup's default.
Expand All @@ -235,7 +230,7 @@ This will become a hard error in the future.`,
},

resolveImportMeta(property) {
if (isEsmOutput) return;
if (isEsmOutput()) return;

if (property === 'url') {
// In our loader, `module.uri` is already fully resolved
Expand All @@ -246,7 +241,7 @@ This will become a hard error in the future.`,

renderChunk(code, chunk, outputOptions) {
// We don’t need to do any loader processing when targeting ESM format.
if (isEsmOutput) return;
if (isEsmOutput()) return;

if (outputOptions.banner && outputOptions.banner.length > 0) {
this.error(
Expand All @@ -256,6 +251,33 @@ This will become a hard error in the future.`,
}
const ms = new MagicString(code);

tippex.match(code, workerRegexpForOutput, (fullMatch, optionsWithCommaStr, optionsStr) => {
let options;
try {
options = json5.parse(optionsStr);
} catch (e) {
// If we couldn't parse the options object, maybe it's something dynamic or has nested
// parentheses or something like that. In that case, treat it as a warning
// and not a hard error, just like we wouldn't break on unmatched regex.
console.warn("Couldn't match options object", fullMatch, ": ", e);
return;
}
if (!("type" in options)) {
// Nothing to do.
return;
}
delete options.type;
const replacementEnd = workerRegexpForOutput.lastIndex;
const replacementStart = replacementEnd - optionsWithCommaStr.length;
optionsStr = json5.stringify(options);
optionsWithCommaStr = optionsStr === "{}" ? "" : `, ${optionsStr}`;
ms.overwrite(
replacementStart,
replacementEnd,
optionsWithCommaStr
);
});

// Mangle define() call
ms.remove(0, "define(".length);
// If the module does not have any dependencies, it’s technically okay
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
},
"dependencies": {
"ejs": "^2.6.1",
"json5": "^2.2.0",
"magic-string": "^0.25.0",
"tippex": "^3.0.0"
}
Expand Down
7 changes: 5 additions & 2 deletions tests/fixtures/module-worker/a.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
* limitations under the License.
*/

export default async function() {
const { default: f } = await import("./b.js");
export default async function(f) {
self.postMessage(f());
}

export function someValue() {
return self.String("a");
}
6 changes: 5 additions & 1 deletion tests/fixtures/module-worker/b.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
* limitations under the License.
*/

// Import for side-effects (of which there are none)
// to make sure that a.js gets its own chunk.
import {someValue} from "./a.js";

export default function() {
return "a";
return someValue();
}
6 changes: 3 additions & 3 deletions tests/fixtures/module-worker/worker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
*/

import a from "./a.js";

a();
import("./b.js");
import("./b.js").then(({default: f}) => {
a(f);
})

0 comments on commit b6cc855

Please sign in to comment.