Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add context option to webpack config #479

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"eslint-plugin-ember": "^5.0.0",
"eslint-plugin-node": "^6.0.1",
"eslint-plugin-prettier": "^3.3.0",
"prettier": "^2.2.1"
"prettier": "^2.2.1",
"webpack-virtual-modules": "^0.4.3"
},
"volta": {
"node": "14.16.0",
Expand Down
38 changes: 36 additions & 2 deletions packages/ember-auto-import/ts/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { PackageCache } from '@embroider/shared-internals';
import { Memoize } from 'typescript-memoize';
import makeDebug from 'debug';
import { ensureDirSync, symlinkSync, existsSync } from 'fs-extra';
import VirtualModulesPlugin from 'webpack-virtual-modules';

const debug = makeDebug('ember-auto-import:webpack');

Expand Down Expand Up @@ -88,6 +89,8 @@ export default class WebpackBundler extends Plugin implements Bundler {

private lastBuildResult: BuildResult | undefined;

private virtualModules;

constructor(priorTrees: InputNode[], private opts: BundlerOptions) {
super(priorTrees, {
persistentOutput: true,
Expand Down Expand Up @@ -127,8 +130,12 @@ export default class WebpackBundler extends Plugin implements Bundler {
entry[bundle] = [
join(stagingDir, 'l.js'),
join(stagingDir, `${bundle}.js`),
// If I understand correctly, our virtual modules will need to go in here...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, the virtual modules would replace the existing join(stagingDir, ...) lines.

// `./__ember_auto_import__/l.js`,
// `./__ember_auto_import__/${bundle}.js`,
];
});

let config: Configuration = {
mode:
this.opts.environment === 'production' ? 'production' : 'development',
Expand Down Expand Up @@ -172,7 +179,9 @@ export default class WebpackBundler extends Plugin implements Bundler {
),
},
module: {
noParse: (file: string) => file === join(stagingDir, 'l.js'),
// I guess webpack should also be prevented from parsing our virtual modules, so presumably we could
// filter them out like this?
noParse: (file: string) => file === join(stagingDir, 'l.js') || file === `./__ember_auto_import__/l.js`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and this would now only need to match the l.js in the virtual modules.

this.stagingDir should be able to be entirely refactored away.

rules: [
this.babelRule(stagingDir),
{
Expand All @@ -196,6 +205,10 @@ export default class WebpackBundler extends Plugin implements Bundler {
},
node: false,
externals: this.externalsHandler,
// Here we add our virtual modules as per https://github.com/sysgears/webpack-virtual-modules
plugins: [
this.virtualModules
]
};

mergeConfig(
Expand Down Expand Up @@ -232,7 +245,10 @@ export default class WebpackBundler extends Plugin implements Bundler {
//
// And we otherwise defer to the `skipBabel` setting as implemented by
// `@embroider/shared-internals`.
return dirname(filename) !== stagingDir && shouldTranspile(filename);
return dirname(filename) !== stagingDir
// I presume we also don't want to apply babel to our virtual modules, which I guess could be done as follows...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

&& filename.indexOf('/__ember_auto_import__/')<0
&& shouldTranspile(filename);
},
use: {
loader: 'babel-loader-8',
Expand Down Expand Up @@ -289,10 +305,27 @@ export default class WebpackBundler extends Plugin implements Bundler {
async build(): Promise<void> {
let bundleDeps = await this.opts.splitter.deps();

// Build our virtual modules which we'll pass into the webpack plugins array
let virtualModulesHash = {};
for (let [bundle, deps] of bundleDeps.entries()) {
virtualModulesHash[`./__ember_auto_import__/${bundle}.js`] = entryTemplate({
staticImports: deps.staticImports,
dynamicImports: deps.dynamicImports,
dynamicTemplateImports:
deps.dynamicTemplateImports.map(mapTemplateImports),
staticTemplateImports:
deps.staticTemplateImports.map(mapTemplateImports),
publicAssetURL: this.opts.publicAssetURL,
});
}
virtualModulesHash[`./__ember_auto_import__/l.js`] = loader;
this.virtualModules = new VirtualModulesPlugin(virtualModulesHash);

for (let [bundle, deps] of bundleDeps.entries()) {
this.writeEntryFile(bundle, deps);
}
this.writeLoaderFile();

this.linkDeps(bundleDeps);
let stats = await this.runWebpack();
this.lastBuildResult = this.summarizeStats(stats, bundleDeps);
Expand Down Expand Up @@ -391,6 +424,7 @@ export default class WebpackBundler extends Plugin implements Bundler {
packageName: string;
packageRoot: string;
}): void {
// I guess this is "part 2" of what stagingDir is being used for. Not yet sure how to replace this part...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The files like ./__ember_auto_import__/${bundle}.js contain imports, and webpack will try to resolve those imported packages relative to that path. It needs to find the right ones. In the stagingDir we did that by making symlinks.

You could instead use resolve.alias or resolve.fallback. See https://webpack.js.org/configuration/resolve/ This is probably a small amount of code to write, but it could have confusing effects on people who were already doing some aliasing.

The alternative I mentioned in #479 (comment) is to actually locate the virtual modules such that they appear to be inside the real package that did the importing, so that the existing dependency resolution would work. This would be very nice because it means there's no confusion about aliases and we'd be closer to how a "normal" webpack build would work (one where webpack looks at the actual app source, instead of ember-auto-import's summary of the app source). This might need wider code changes though, so that you could know the needed imports on a per-package (meaning the app and any addons that are using ember-auto-import) basis.

ensureDirSync(dirname(join(this.stagingDir, 'node_modules', packageName)));
if (!existsSync(join(this.stagingDir, 'node_modules', packageName))) {
symlinkSync(
Expand Down
2 changes: 1 addition & 1 deletion test-scenarios/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"jsdom": "^11.11.0"
},
"scripts": {
"test": "qunit --require ts-node/register *-test.ts",
"test": "qunit --require ts-node/register *-test.ts --filter lts-common-chunk",
"test:list": "scenario-tester list --require ts-node/register --files=*-test.ts",
"test:output": "scenario-tester output --require ts-node/register --files=*-test.ts"
},
Expand Down