Skip to content

Commit

Permalink
Restructure entry file bundle logic to support "layering"
Browse files Browse the repository at this point in the history
Currently, ember-auto-import has two conceptual buckets where it places the imports it has found (an app bucket and a tests bucket). While this works, it has two potential problems. The first is with imports from within lazy engines as they get placed inside of the "app" bucket. This means that instead of these asset loading lazily they are instead eagerly loaded as part of the main app bundle. The second problem is that modules can be "duplicated" in each bucket and can cause problems with module states' being different depending on the importer.

In order to fix this, this PR creates an individual entry file (or bucket) for every addon that is doing an import. It then creates import links from each entry file to its rightful parent (ie a nested addon from a lazy engine would only be imported from the lazy engine's entry file and then the lazy engine's entry file is import from the app's entry file). Additionally, the tests entry file now imports the app entry file which creates a conceptual graph for webpack to properly traverse such that modules will not be duplicated.

Fixes: embroider-build#503
  • Loading branch information
thoov committed Feb 10, 2023
1 parent 29eab60 commit 061ec72
Show file tree
Hide file tree
Showing 8 changed files with 753 additions and 59 deletions.
339 changes: 333 additions & 6 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/ember-auto-import/ts/auto-import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export default class AutoImport implements AutoImportSharedAPI {
this.packages.add(Package.lookupParentOf(topmostAddon));
let host = topmostAddon.app;
this.env = host.env;
this.bundles = new BundleConfig(host.options.outputPaths);
this.bundles = new BundleConfig(host.options.outputPaths, host);
if (!this.env) {
throw new Error('Bug in ember-auto-import: did not discover environment');
}
Expand Down
7 changes: 6 additions & 1 deletion packages/ember-auto-import/ts/bundle-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import { dirname } from 'path';
import { AppInstance } from '@embroider/shared-internals';
const testsPattern = new RegExp(`^(@[^/]+)?/?[^/]+/(tests|test-support)/`);

import type { TreeType } from './analyzer';
Expand All @@ -26,14 +27,18 @@ interface OutputPaths {
}

export default class BundleConfig {
constructor(private outputPaths: OutputPaths) {}
constructor(private outputPaths: OutputPaths, private host?: AppInstance) {}

// This list of valid bundles, in priority order. The first one in the list that
// needs a given import will end up with that import.
get names(): ReadonlyArray<BundleName> {
return Object.freeze(['app', 'tests']);
}

get hostProject() {
return this.host!.project;
}

isBuiltInBundleName(name: string): name is BundleName {
return this.names.includes(name as BundleName);
}
Expand Down
18 changes: 16 additions & 2 deletions packages/ember-auto-import/ts/inserter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ export class Inserter extends Plugin {
let ast = parse5.parse(html, { sourceCodeLocationInfo: true });
let stringInserter = new StringInserter(html);

// make sure we do not duplicate scripts that would
// be added to the test html file.
let insertedScriptAssets: Set<string> = new Set();

if (this.options.insertScriptsAt) {
debug(
`looking for custom script element: %s`,
Expand Down Expand Up @@ -160,7 +164,8 @@ export class Inserter extends Plugin {
fastbootInfo,
stringInserter,
element,
src
src,
insertedScriptAssets
);
}
}
Expand Down Expand Up @@ -233,14 +238,23 @@ export class Inserter extends Plugin {
fastbootInfo: ReturnType<typeof Inserter.prototype.fastbootManifestInfo>,
stringInserter: StringInserter,
element: parse5.Element,
src: string
src: string,
insertedScriptAssets: Set<string>
) {
for (let entry of targets.scripts) {
if (entry.afterFile && src.endsWith(entry.afterFile)) {
let { scriptChunks, bundleName } = entry;
entry.inserted = true;
debug(`inserting %s`, scriptChunks);
let insertedSrc = scriptChunks
.filter((s) => {
if (insertedScriptAssets.has(s)) {
return false;
}

insertedScriptAssets.add(s);
return true;
})
.map((chunk) => `\n<script src="${this.chunkURL(chunk)}"></script>`)
.join('');
if (fastbootInfo?.readsHTML && bundleName === 'app') {
Expand Down
83 changes: 52 additions & 31 deletions packages/ember-auto-import/ts/splitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import makeDebug from 'debug';
import Analyzer, { Import, LiteralImport, TemplateImport } from './analyzer';
import Package, { DepResolution } from './package';
import { shallowEqual } from './util';
import { flatten, partition } from 'lodash';
import { flatten } from 'lodash';
import BundleConfig from './bundle-config';
import { join } from 'path';
import { satisfies } from 'semver';
Expand Down Expand Up @@ -234,42 +234,50 @@ export default class Splitter {
let deps: Map<string, BundleDependencies> = new Map();

this.options.bundles.names.forEach((bundleName) => {
deps.set(bundleName, {
staticImports: [],
staticTemplateImports: [],
dynamicImports: [],
dynamicTemplateImports: [],
});
getOrCreate(deps, bundleName);
});

for (let target of targets.targets.values()) {
let [dynamicUses, staticUses] = partition(
target.importedBy,
(imp) => imp.isDynamic
);
if (staticUses.length > 0) {
let bundleName = this.chooseBundle(staticUses);
deps.get(bundleName)!.staticImports.push(target);
}
if (dynamicUses.length > 0) {
let bundleName = this.chooseBundle(dynamicUses);
deps.get(bundleName)!.dynamicImports.push(target);
}
target.importedBy.forEach((i) => {
let bundleName = this.chooseBundle([i]);

if (bundleName === 'tests') {
if (i.isDynamic) {
getOrCreate(deps, 'tests').dynamicImports.push(target);
} else {
getOrCreate(deps, 'tests').staticImports.push(target);
}
} else {
let depName = i.package.isAddon ? i.package.name : 'app';
if (i.isDynamic) {
getOrCreate(deps, depName).dynamicImports.push(target);
} else {
getOrCreate(deps, depName).staticImports.push(target);
}
}
});
}

for (let target of targets.templateTargets.values()) {
let [dynamicUses, staticUses] = partition(
target.importedBy,
(imp) => imp.isDynamic
);
if (staticUses.length > 0) {
let bundleName = this.chooseBundle(staticUses);
deps.get(bundleName)!.staticTemplateImports.push(target);
}
if (dynamicUses.length > 0) {
let bundleName = this.chooseBundle(dynamicUses);
deps.get(bundleName)!.dynamicTemplateImports.push(target);
}
target.importedBy.forEach((i) => {
let bundleName = this.chooseBundle([i]);

if (bundleName === 'tests') {
if (i.isDynamic) {
getOrCreate(deps, 'tests').dynamicTemplateImports.push(target);
} else {
getOrCreate(deps, 'tests').staticTemplateImports.push(target);
}
} else {
let depName = i.package.isAddon ? i.package.name : 'app';

if (i.isDynamic) {
getOrCreate(deps, depName).dynamicTemplateImports.push(target);
} else {
getOrCreate(deps, depName).staticTemplateImports.push(target);
}
}
});
}

this.sortDependencies(deps);
Expand Down Expand Up @@ -367,3 +375,16 @@ class LazyPrintDeps {
return JSON.stringify(output, null, 2);
}
}

function getOrCreate(deps: Map<string, BundleDependencies>, name: string) {
if (!deps.has(name)) {
deps.set(name, {
staticImports: [],
staticTemplateImports: [],
dynamicImports: [],
dynamicTemplateImports: [],
});
}

return deps.get(name)!;
}
112 changes: 94 additions & 18 deletions packages/ember-auto-import/ts/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ import { Options } from './package';
import { PackageCache } from '@embroider/shared-internals';
import { Memoize } from 'typescript-memoize';
import makeDebug from 'debug';
import { ensureDirSync, symlinkSync, existsSync } from 'fs-extra';
import {
ensureDirSync,
symlinkSync,
existsSync,
outputFileSync,
} from 'fs-extra';
import MiniCssExtractPlugin from 'mini-css-extract-plugin';
import semver from 'semver';

Expand Down Expand Up @@ -90,6 +95,16 @@ module.exports = (function(){
{{! this is only used for synchronous importSync() using a template string }}
return r('_eai_sync_' + specifier)(Array.prototype.slice.call(arguments, 1))
};
{{#each relativeImports as |module|}}
require('./{{js-string-escape module}}.cjs');
{{/each}}
{{#each lazyEngineImports as |module|}}
var engineLookup = window.__eaiEngineLookup || {};
engineLookup['{{module}}'] = function() {
return import('./{{js-string-escape module}}.cjs')
};
window.__eaiEngineLookup = engineLookup;
{{/each}}
d('__v1-addons__early-boot-set__', [{{{v1EmberDeps}}}], function() {});
{{#each staticImports as |module|}}
d('{{js-string-escape module.specifier}}', ['__v1-addons__early-boot-set__'], function() { return require('{{js-string-escape module.specifier}}'); });
Expand Down Expand Up @@ -119,10 +134,18 @@ module.exports = (function(){
dynamicImports: { specifier: string }[];
staticTemplateImports: { key: string; args: string; template: string }[];
dynamicTemplateImports: { key: string; args: string; template: string }[];
relativeImports: { specifier: string }[];
lazyEngineImports: { specifier: string }[];
publicAssetURL: string | undefined;
v1EmberDeps: string;
}) => string;

const emptyTemplate = compile(`
{{#each relativeImports as |module|}}
require('./{{js-string-escape module}}.cjs');
{{/each}}
`) as (args: { relativeImports: { specifier: string }[] }) => string;

// this goes in a file by itself so we can tell webpack not to parse it. That
// allows us to grab the "require" and "define" from our enclosing scope without
// webpack messing with them.
Expand Down Expand Up @@ -209,6 +232,7 @@ export default class WebpackBundler extends Plugin implements Bundler {
library: '__ember_auto_import__',
},
optimization: {
removeAvailableModules: true,
splitChunks: {
chunks: 'all',
},
Expand Down Expand Up @@ -377,15 +401,46 @@ export default class WebpackBundler extends Plugin implements Bundler {
async build(): Promise<void> {
let bundleDeps = await this.opts.splitter.deps();

for (let [bundle, deps] of bundleDeps.entries()) {
this.writeEntryFile(bundle, deps);
}
this.recursivelyCreateEntryFiles(
this.opts.bundles.hostProject,
bundleDeps,
true
);
this.writeLoaderFile();
this.linkDeps(bundleDeps);
let stats = await this.runWebpack();
this.lastBuildResult = this.summarizeStats(stats, bundleDeps);
}

private recursivelyCreateEntryFiles(project: any, deps: any, isApp: boolean) {
let addonNames: string[] = [];
let lazyEngineNames: string[] = [];

project.addons.forEach((addon: any) => {
if (
addon.options &&
addon.options.lazyLoading &&
addon.options.lazyLoading.enabled
) {
lazyEngineNames.push(flattenFileName(addon.name));
} else if (addon.addons.length) {
addonNames.push(flattenFileName(addon.name));
}

if (addon.addons.length) {
this.recursivelyCreateEntryFiles(addon, deps, false);
}
});

if (isApp) {
this.writeEntryFile('app', deps.get('app')!, addonNames, lazyEngineNames);
this.writeEntryFile('tests', deps.get('tests')!, ['app'], []);
} else {
let name = project.name;
this.writeEntryFile(name, deps.get(name), addonNames, lazyEngineNames);
}
}

private summarizeStats(
_stats: Required<Stats>,
bundleDeps: Map<string, BundleDependencies>
Expand Down Expand Up @@ -530,22 +585,38 @@ export default class WebpackBundler extends Plugin implements Bundler {
return result;
}

private writeEntryFile(name: string, deps: BundleDependencies) {
private writeEntryFile(
name: string,
deps: BundleDependencies,
relativeImports: any,
lazyEngineImports: any
) {
let v1EmberDeps = this.getEarlyBootSet();

writeFileSync(
join(this.stagingDir, `${name}.cjs`),
entryTemplate({
staticImports: deps.staticImports,
dynamicImports: deps.dynamicImports,
dynamicTemplateImports:
deps.dynamicTemplateImports.map(mapTemplateImports),
staticTemplateImports:
deps.staticTemplateImports.map(mapTemplateImports),
publicAssetURL: this.opts.publicAssetURL,
v1EmberDeps: v1EmberDeps.map((name) => `'${name}'`).join(','),
})
);
if (!existsSync(join(this.stagingDir, name))) {
if (!deps) {
outputFileSync(
join(this.stagingDir, `${flattenFileName(name)}.cjs`),
emptyTemplate({ relativeImports })
);
} else {
writeFileSync(
join(this.stagingDir, `${name}.cjs`),
entryTemplate({
staticImports: deps.staticImports,
dynamicImports: deps.dynamicImports,
dynamicTemplateImports:
deps.dynamicTemplateImports.map(mapTemplateImports),
staticTemplateImports:
deps.staticTemplateImports.map(mapTemplateImports),
relativeImports,
lazyEngineImports,
publicAssetURL: this.opts.publicAssetURL,
v1EmberDeps: v1EmberDeps.map((name) => `'${name}'`).join(','),
})
);
}
}
}

private writeLoaderFile() {
Expand Down Expand Up @@ -692,3 +763,8 @@ function nonEmptyBundle(
function removeUndefined<T>(list: (T | undefined)[]): T[] {
return list.filter((item) => typeof item !== 'undefined') as T[];
}

function flattenFileName(path: string) {
let re = new RegExp('/', 'g');
return path.replace(re, '-');
}
Loading

0 comments on commit 061ec72

Please sign in to comment.