Skip to content

Commit

Permalink
Use empty source map with worker-loader
Browse files Browse the repository at this point in the history
Reverting a hack that was implemented in https://github.com/uber/fusionjs/pull/1489 to fix a module name collision in generated source maps, as modifying the import path by appending querystring is not compatible with the resolution logic specified for "exports" field, where the package consumers must use exact import paths according to how the package author has it defined in the "exports" field in the package.json. E.g., for [@duckdb/duckdb-wasm](https://github.com/duckdb/duckdb-wasm/blob/15c8075b5f82d3d32386c5c1cd425b5473524de6/packages/duckdb-wasm/package.json#L96-L146)
```
{
  "...": "",
  "exports": {
        "./dist/duckdb-browser-mvp.worker.js": "./dist/duckdb-browser-mvp.worker.js",
        "...": ""
  }
}
```

The import must take form of `import from '@duckdb/duckdb-wasm/dist/duckdb-browser-mvp.worker.js'`, and `import from '@duckdb/duckdb-wasm/dist/duckdb-browser-mvp.worker.js?workerUrl=true'` is not allowed

Here's Node.js's `require.resolve()` output:
```
Welcome to Node.js v16.15.0.

> require.resolve('@duckdb/duckdb-wasm/dist/duckdb-browser-mvp.worker.js')
'~/test-dd/node_modules/@duckdb/duckdb-wasm/dist/duckdb-browser-mvp.worker.js'

> require.resolve('@duckdb/duckdb-wasm/dist/duckdb-browser-mvp.worker.js?workerUrl=true')
Uncaught:
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './dist/duckdb-browser-mvp.worker.js?workerUrl=true' is not defined by "exports" in ~/test-dd/node_modules/@duckdb/duckdb-wasm/package.json
    at __node_internal_captureLargerStackTrace (node:internal/errors:465:5)
    at new NodeError (node:internal/errors:372:5)
    at throwExportsNotFound (node:internal/modules/esm/resolve:472:9)
    at packageExportsResolve (node:internal/modules/esm/resolve:753:3)
    at resolveExports (node:internal/modules/cjs/loader:482:36)
    at Function.Module._findPath (node:internal/modules/cjs/loader:522:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:919:27)
    at Function.resolve (node:internal/modules/cjs/helpers:108:19) {
  code: 'ERR_PACKAGE_PATH_NOT_EXPORTED'
}
```

The original issue seems to be fixable by resolving the `worker-loader` using empty source map for the "virtual" module it generates.
  • Loading branch information
shYkiSto authored and fusionjs-sync-bot[bot] committed Jul 28, 2022
1 parent 91783e2 commit 3764d62
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 7 deletions.
40 changes: 40 additions & 0 deletions fusion-cli-tests/test/e2e/worker/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* eslint-env node */

const t = require('assert');
const fs = require('fs');
const path = require('path');

const puppeteer = require('puppeteer');
Expand Down Expand Up @@ -46,3 +47,42 @@ test('`fusion build` app with worker integration', async () => {
await browser.close();
proc.kill('SIGKILL');
}, 100000);

test('`fusion build` worker loader client source maps', async () => {
const dir = path.resolve(__dirname + '/fixture');

const env = Object.create(process.env);
env.NODE_ENV = 'production';

await cmd(`build --dir=${dir} --production --modernBuildOnly`, {env});

const clientDistPath = path.join(
dir,
'.fusion',
'dist',
'production',
'client'
);

const clientSourceMapFiles = (
await fs.promises.readdir(clientDistPath)
).filter((filePath) => filePath.endsWith('.map'));

const sourceMaps = (
await Promise.all(
clientSourceMapFiles.map((filePath) =>
fs.promises.readFile(path.join(clientDistPath, filePath), 'utf-8')
)
)
).map((fileContents) => JSON.parse(fileContents));

const sources = sourceMaps.reduce((acc, sourceMap) => {
return [...acc, ...sourceMap.sources];
}, []);
const uniqueSources = new Set(sources);

t(
uniqueSources.size === sources.length,
'Generated source maps should not contain duplicate entries for the same source file'
);
}, 100000);
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ function refsHandler(t, context, refs = []) {
}
args[0].replaceWith(
t.callExpression(t.identifier('require'), [
t.stringLiteral(
`__SECRET_WORKER_LOADER__!${args[0].node.value}?workerUrl=true`
),
t.stringLiteral(`__SECRET_WORKER_LOADER__!${args[0].node.value}`),
])
);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { workerUrl } from 'fusion-core';
import { workerUrl as workerUrlOther } from 'workerUrl';
const path = './test';
workerUrl(require("__SECRET_WORKER_LOADER__!./path?workerUrl=true"));
workerUrl(require("__SECRET_WORKER_LOADER__!./path"));
workerUrlOther(path);
workerUrl(require("__SECRET_WORKER_LOADER__!./path?workerUrl=true"));
workerUrl(require("__SECRET_WORKER_LOADER__!./path"));
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { workerUrl as frameworkWorkerUrl } from 'fusion-core';
import workerUrl from 'workerURL';
workerUrl('./path');
frameworkWorkerUrl(require("__SECRET_WORKER_LOADER__!./path?workerUrl=true"));
frameworkWorkerUrl(require("__SECRET_WORKER_LOADER__!./path"));
18 changes: 17 additions & 1 deletion fusion-cli/build/loaders/worker-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ class WorkerLoaderError extends Error {
}
}

const EMPTY_SOURCE_MAP = {
file: 'x',
version: 3,
names: [],
sources: [],
sourcesContent: [],
mappings: '',
};

const getWorker = (file, content, options) => {
const publicPath = options.publicPath
? JSON.stringify(options.publicPath)
Expand Down Expand Up @@ -122,7 +131,14 @@ module.exports.pitch = function (request /* : any*/) {
delete this._compilation.assets[worker.file];
}

return cb(null, `module.exports = ${worker.factory};\n`);
return cb(
null,
`module.exports = ${worker.factory};\n`,
// Returning an empty source map for the "virtual" module created here,
// fixes a naming collision with the actual source map generated for the
// bundled web worker script
EMPTY_SOURCE_MAP
);
}

return cb(null, null);
Expand Down

0 comments on commit 3764d62

Please sign in to comment.