Skip to content

Commit

Permalink
Correctly set worker's output format if not support by environment (p…
Browse files Browse the repository at this point in the history
  • Loading branch information
mischnic authored Jul 2, 2021
1 parent c999ae1 commit 8d50ec6
Show file tree
Hide file tree
Showing 7 changed files with 766 additions and 12 deletions.
12 changes: 10 additions & 2 deletions packages/core/core/src/requests/TargetRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,9 @@ export class TargetResolver {
context: descriptor.context,
isLibrary: descriptor.isLibrary,
includeNodeModules: descriptor.includeNodeModules,
outputFormat: descriptor.outputFormat,
outputFormat:
descriptor.outputFormat ??
this.options.defaultTargetOptions.outputFormat,
shouldOptimize:
this.options.defaultTargetOptions.shouldOptimize &&
descriptor.optimize !== false,
Expand Down Expand Up @@ -291,6 +293,7 @@ export class TargetResolver {
context: 'browser',
engines: {},
shouldOptimize: this.options.defaultTargetOptions.shouldOptimize,
outputFormat: this.options.defaultTargetOptions.outputFormat,
shouldScopeHoist: this.options.defaultTargetOptions
.shouldScopeHoist,
sourceMap: this.options.defaultTargetOptions.sourceMaps
Expand Down Expand Up @@ -590,6 +593,7 @@ export class TargetResolver {

let outputFormat =
descriptor.outputFormat ??
this.options.defaultTargetOptions.outputFormat ??
inferredOutputFormat ??
(targetName === 'module' ? 'esmodule' : 'commonjs');
let isModule = outputFormat === 'esmodule';
Expand Down Expand Up @@ -753,7 +757,10 @@ export class TargetResolver {
context: descriptor.context,
includeNodeModules: descriptor.includeNodeModules,
outputFormat:
descriptor.outputFormat ?? inferredOutputFormat ?? undefined,
descriptor.outputFormat ??
this.options.defaultTargetOptions.outputFormat ??
inferredOutputFormat ??
undefined,
isLibrary: descriptor.isLibrary,
shouldOptimize:
this.options.defaultTargetOptions.shouldOptimize &&
Expand All @@ -779,6 +786,7 @@ export class TargetResolver {
env: createEnvironment({
engines: pkgEngines,
context,
outputFormat: this.options.defaultTargetOptions.outputFormat,
shouldOptimize: this.options.defaultTargetOptions.shouldOptimize,
shouldScopeHoist: this.options.defaultTargetOptions.shouldScopeHoist,
sourceMap: this.options.defaultTargetOptions.sourceMaps
Expand Down
1 change: 1 addition & 0 deletions packages/core/core/src/resolveOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export default async function resolveOptions(
publicUrl,
distDir,
engines: initialOptions?.defaultTargetOptions?.engines,
outputFormat: initialOptions?.defaultTargetOptions?.outputFormat,
},
};
}
1 change: 1 addition & 0 deletions packages/core/core/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ export type ParcelOptions = {|
+publicUrl: string,
+distDir?: FilePath,
+engines?: Engines,
+outputFormat?: OutputFormat,
|},
|};

Expand Down

Large diffs are not rendered by default.

85 changes: 80 additions & 5 deletions packages/core/integration-tests/test/javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -851,15 +851,17 @@ describe('javascript', function() {
assert(/new SharedWorker(.*?, {[\n\s]+type: 'module'[\n\s]+})/.test(main));
});

for (let scopeHoist of [true, false]) {
for (let shouldScopeHoist of [true, false]) {
it(`should compile workers to non modules if ${
scopeHoist ? 'browsers do not support it' : 'scopeHoist = false'
shouldScopeHoist
? 'browsers do not support it'
: 'shouldScopeHoist = false'
}`, async function() {
let b = await bundle(
path.join(__dirname, '/integration/workers-module/index.js'),
{
defaultTargetOptions: {
shouldScopeHoist: true,
shouldScopeHoist,
engines: {
browsers: '>= 0.25%',
},
Expand All @@ -869,14 +871,22 @@ describe('javascript', function() {

assertBundles(b, [
{
assets: ['dedicated-worker.js', 'index.js'],
assets: [
'dedicated-worker.js',
!shouldScopeHoist && 'esmodule-helpers.js',
'index.js',
].filter(Boolean),
},
{
name: 'index.js',
assets: ['index.js', 'bundle-url.js', 'get-worker-url.js'],
},
{
assets: ['shared-worker.js', 'index.js'],
assets: [
!shouldScopeHoist && 'esmodule-helpers.js',
'shared-worker.js',
'index.js',
].filter(Boolean),
},
]);

Expand Down Expand Up @@ -905,6 +915,71 @@ describe('javascript', function() {
});
}

for (let supported of [false, true]) {
it(`should compile workers to ${
supported ? '' : 'non '
}modules when browsers do ${
supported ? '' : 'not '
}support it with esmodule parent script`, async function() {
let b = await bundle(
path.join(__dirname, '/integration/workers-module/index.js'),
{
mode: 'production',
defaultTargetOptions: {
engines: {browsers: supported ? 'Chrome 80' : 'Chrome 75'},
outputFormat: 'esmodule',
shouldScopeHoist: true,
shouldOptimize: false,
},
},
);

assertBundles(b, [
{
type: 'js',
assets: ['dedicated-worker.js'],
},
{
name: 'index.js',
assets: ['index.js', 'bundle-url.js', 'get-worker-url.js'],
},
{
type: 'js',
assets: ['shared-worker.js'],
},
{
type: 'js',
assets: ['index.js'],
},
]);

let dedicated, shared;
b.traverseBundles((bundle, ctx, traversal) => {
if (bundle.getMainEntry()?.filePath.endsWith('shared-worker.js')) {
shared = bundle;
} else if (
bundle.getMainEntry()?.filePath.endsWith('dedicated-worker.js')
) {
dedicated = bundle;
}
if (dedicated && shared) traversal.stop();
});

assert(dedicated);
assert(shared);

let main = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
assert(/new Worker([^,]*?)/.test(main));
assert(/new SharedWorker([^,]*?)/.test(main));

dedicated = await outputFS.readFile(dedicated.filePath, 'utf8');
shared = await outputFS.readFile(shared.filePath, 'utf8');
let importRegex = supported ? /importScripts\s*\(/ : /import\s*("|')/;
assert(!importRegex.test(dedicated));
assert(!importRegex.test(shared));
});
}

it('should preserve the name option to workers', async function() {
let b = await bundle(
path.join(__dirname, '/integration/workers-module/named.js'),
Expand Down
1 change: 1 addition & 0 deletions packages/core/types/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ export type InitialParcelOptions = {|
+publicUrl?: string,
+distDir?: FilePath,
+engines?: Engines,
+outputFormat?: OutputFormat,
|},

+additionalReporters?: Array<{|
Expand Down
11 changes: 7 additions & 4 deletions packages/transformers/js/src/JSTransformer.js
Original file line number Diff line number Diff line change
Expand Up @@ -484,15 +484,18 @@ export default (new Transformer({
for (let dep of dependencies) {
if (dep.kind === 'WebWorker') {
// Use native ES module output if the worker was created with `type: 'module'` and all targets
// support native module workers. Only do this if the source type is changing from script to module
// though so that assets can be shared between workers and the main thread in the global output format.
let outputFormat = asset.env.outputFormat;
// support native module workers. Only do this if parent asset output format is also esmodule so that
// assets can be shared between workers and the main thread in the global output format.
let outputFormat;
if (
asset.env.sourceType === 'script' &&
asset.env.outputFormat === 'esmodule' &&
dep.source_type === 'Module' &&
supportsModuleWorkers
) {
outputFormat = 'esmodule';
} else {
outputFormat =
asset.env.outputFormat === 'commonjs' ? 'commonjs' : 'global';
}

let loc = convertLoc(dep.loc);
Expand Down

0 comments on commit 8d50ec6

Please sign in to comment.