From 6c43934a0c39bd2fd7230c1ad8610bad26a77c25 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Wed, 24 Mar 2021 11:59:18 -0400 Subject: [PATCH 1/2] Add a failing test for common chunk bug This reproduces issue #365. --- test-scenarios/common-chunk-test.ts | 101 ++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 test-scenarios/common-chunk-test.ts diff --git a/test-scenarios/common-chunk-test.ts b/test-scenarios/common-chunk-test.ts new file mode 100644 index 000000000..79ca6b940 --- /dev/null +++ b/test-scenarios/common-chunk-test.ts @@ -0,0 +1,101 @@ +import { appScenarios } from './scenarios'; +import { PreparedApp } from '@ef4/test-support'; +import QUnit from 'qunit'; +import merge from 'lodash/merge'; +const { module: Qmodule, test } = QUnit; + +appScenarios + .map('common-chunk', project => { + project.linkDevDependency('ember-auto-import', { baseDir: __dirname }); + + project.addDevDependency('some-lib', { + files: { + 'left.js': ` + import { helper } from './common'; + export function left() { + return helper('left'); + } + `, + 'right.js': ` + import { helper } from './common'; + export function right() { + return helper('right'); + } + `, + 'common.js': ` + export function helper(msg) { + return "the message is " + msg; + } + `, + }, + }); + + merge(project.files, { + 'ember-cli-build.js': ` + const EmberApp = require('ember-cli/lib/broccoli/ember-app'); + module.exports = function (defaults) { + let app = new EmberApp(defaults, { + babel: { + plugins: [ + require('ember-auto-import/babel-plugin') + ], + }, + autoImport: { + webpack: { + optimization: { + splitChunks: { + // for test purposes, we want chunk + // splitting even though our common chunk is very small + minSize: 0 + } + } + } + } + }); + return app.toTree(); + }; + `, + app: { + lib: { + 'example.js': ` + export async function useLeft() { + let { left } = await import('some-lib/left'); + return left(); + } + export async function useRight() { + let { right } = await import('some-lib/right'); + return right(); + } + `, + }, + }, + tests: { + unit: { + 'example-test.js': ` + import { module, test } from 'qunit'; + import { useLeft, useRight } from '@ef4/app-template/lib/example'; + + module('Unit | common chunk', function () { + test('can use two dynamic imports that share a common chunk', async function(assert) { + assert.equal(await useLeft(), 'The message is left'); + assert.equal(await useRight(), 'The message is right'); + }); + }); + `, + }, + }, + }); + }) + .forEachScenario(scenario => { + Qmodule(scenario.name, function (hooks) { + let app: PreparedApp; + hooks.before(async () => { + app = await scenario.prepare(); + }); + + test('npm run test', async function (assert) { + let result = await app.execute('npm run test'); + assert.equal(result.exitCode, 0, result.output); + }); + }); + }); From 9447c388c1a7e5c9c3962fdf7f6c0ab5c5af40af Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Wed, 24 Mar 2021 13:17:01 -0400 Subject: [PATCH 2/2] don't rely on chunk name stability Instead of trying to make all entry chunk names stable, we'll make sure we clean up stale ones before we append them into vendor.js. --- packages/ember-auto-import/ts/bundler.ts | 10 ++-------- packages/ember-auto-import/ts/webpack.ts | 10 +--------- test-scenarios/common-chunk-test.ts | 4 ++-- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/packages/ember-auto-import/ts/bundler.ts b/packages/ember-auto-import/ts/bundler.ts index 9dbb9f718..29ba3e947 100644 --- a/packages/ember-auto-import/ts/bundler.ts +++ b/packages/ember-auto-import/ts/bundler.ts @@ -35,7 +35,6 @@ export interface BundlerHook { export default class Bundler extends Plugin { private lastDeps: Map | undefined; private cachedBundlerHook: BundlerHook | undefined; - private didEnsureDirs: boolean; private options: BundlerPluginOptions; private isWatchingSomeDeps: boolean; @@ -46,7 +45,6 @@ export default class Bundler extends Plugin { needsCache: true, }); this.options = options; - this.didEnsureDirs = false; this.isWatchingSomeDeps = deps.length > 1; } @@ -107,27 +105,23 @@ export default class Bundler extends Plugin { } async build() { - this.ensureDirs(); reloadDevPackages(); let { splitter } = this.options; let bundleDeps = await splitter.deps(); if (bundleDeps !== this.lastDeps || this.isWatchingSomeDeps) { let buildResult = await this.bundlerHook.build(bundleDeps); + this.emptyDirs(); this.addEntrypoints(buildResult); this.addLazyAssets(buildResult); this.lastDeps = bundleDeps; } } - private ensureDirs() { - if (this.didEnsureDirs) { - return; - } + private emptyDirs() { emptyDirSync(join(this.outputPath, 'lazy')); for (let bundle of this.options.bundles.names) { emptyDirSync(join(this.outputPath, 'entrypoints', bundle)); } - this.didEnsureDirs = true; } private addEntrypoints({ entrypoints, dir }: BuildResult) { diff --git a/packages/ember-auto-import/ts/webpack.ts b/packages/ember-auto-import/ts/webpack.ts index 96f6fb934..fee815d9b 100644 --- a/packages/ember-auto-import/ts/webpack.ts +++ b/packages/ember-auto-import/ts/webpack.ts @@ -114,9 +114,7 @@ export default class WebpackBundler implements BundlerHook { }, output: { path: this.outputDir, - // entry chunks need to have stable names, so we can more easily gather - // them all up to append to Ember's vendor.js - filename: `chunk.[id].js`, + filename: `chunk.[id].[chunkhash].js`, chunkFilename: `chunk.[id].[chunkhash].js`, libraryTarget: 'var', library: '__ember_auto_import__', @@ -124,12 +122,6 @@ export default class WebpackBundler implements BundlerHook { optimization: { splitChunks: { chunks: 'all', - cacheGroups: { - // similar to above, entry vendor chunks need to have stable names - vendors: { - filename: 'chunk.[name].js', - } as any, // typings are missing a valid documented option - }, }, }, resolveLoader: { diff --git a/test-scenarios/common-chunk-test.ts b/test-scenarios/common-chunk-test.ts index 79ca6b940..15e2f80b3 100644 --- a/test-scenarios/common-chunk-test.ts +++ b/test-scenarios/common-chunk-test.ts @@ -77,8 +77,8 @@ appScenarios module('Unit | common chunk', function () { test('can use two dynamic imports that share a common chunk', async function(assert) { - assert.equal(await useLeft(), 'The message is left'); - assert.equal(await useRight(), 'The message is right'); + assert.equal(await useLeft(), 'the message is left'); + assert.equal(await useRight(), 'the message is right'); }); }); `,