Skip to content

Commit 5204edf

Browse files
authored
Merge pull request #603 from embroider-build/fix-imports-with-query
Fix imports with a query part
2 parents 8e2c9a8 + 12e5c09 commit 5204edf

File tree

4 files changed

+84
-6
lines changed

4 files changed

+84
-6
lines changed

packages/ember-auto-import/ts/package.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import semver from 'semver';
1313
import type { TransformOptions } from '@babel/core';
1414
import { MacrosConfig } from '@embroider/macros/src/node';
1515
import minimatch from 'minimatch';
16+
import { stripQuery } from './util';
1617

1718
// from child addon instance to their parent package
1819
const parentCache: WeakMap<AddonInstance, Package> = new WeakMap();
@@ -315,7 +316,9 @@ export default class Package {
315316
if (!this.isAddon && packageName === this.name) {
316317
let localPath = path.slice(packageName.length + 1);
317318
if (
318-
this.allowAppImports.some((pattern) => minimatch(localPath, pattern))
319+
this.allowAppImports.some((pattern) =>
320+
minimatch(stripQuery(localPath), pattern)
321+
)
319322
) {
320323
return {
321324
type: 'package',

packages/ember-auto-import/ts/util.ts

+4
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,7 @@ export function shallowEqual(a: any[], b: any[]) {
66
a.every((item, index) => item === b[index])
77
);
88
}
9+
10+
export function stripQuery(path: string) {
11+
return path.split('?')[0];
12+
}

packages/ember-auto-import/ts/webpack.ts

+30-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
Module,
99
} from 'webpack';
1010
import { join, dirname, resolve, relative, posix, isAbsolute } from 'path';
11+
import { createHash } from 'crypto';
1112
import { mergeWith, flatten, zip } from 'lodash';
1213
import { writeFileSync, realpathSync, readFileSync } from 'fs';
1314
import { compile, registerHelper } from 'handlebars';
@@ -25,6 +26,7 @@ import { ensureDirSync, symlinkSync, existsSync } from 'fs-extra';
2526
import MiniCssExtractPlugin from 'mini-css-extract-plugin';
2627
import minimatch from 'minimatch';
2728
import { TransformOptions } from '@babel/core';
29+
import { stripQuery } from './util';
2830

2931
const EXTENSIONS = ['.js', '.ts', '.json'];
3032

@@ -35,6 +37,27 @@ registerHelper('join', function (list, connector) {
3537
return list.join(connector);
3638
});
3739

40+
const moduleIdMap = new Map<string, string>();
41+
42+
function moduleToId(moduleSpecifier: string) {
43+
let id = moduleSpecifier;
44+
45+
// if the module contains characters that need to be escaped, then map this to a hash instead, so we can easily replace this later
46+
if (moduleSpecifier.includes('"') || moduleSpecifier.includes("'")) {
47+
id = createHash('md5').update('some_string').digest('hex');
48+
49+
moduleIdMap.set(id, moduleSpecifier);
50+
}
51+
52+
return id;
53+
}
54+
55+
function idToModule(id: string) {
56+
return moduleIdMap.get(id) ?? id;
57+
}
58+
59+
registerHelper('module-to-id', moduleToId);
60+
3861
const entryTemplate = compile(
3962
`
4063
module.exports = (function(){
@@ -52,7 +75,7 @@ module.exports = (function(){
5275
return r('_eai_sync_' + specifier)(Array.prototype.slice.call(arguments, 1))
5376
};
5477
{{#each staticImports as |module|}}
55-
d('{{js-string-escape module.specifier}}', EAI_DISCOVERED_EXTERNALS('{{js-string-escape module.specifier}}'), function() { return require('{{js-string-escape module.specifier}}'); });
78+
d('{{js-string-escape module.specifier}}', EAI_DISCOVERED_EXTERNALS('{{module-to-id module.specifier}}'), function() { return require('{{js-string-escape module.specifier}}'); });
5679
{{/each}}
5780
{{#each dynamicImports as |module|}}
5881
d('_eai_dyn_{{js-string-escape module.specifier}}', [], function() { return import('{{js-string-escape module.specifier}}'); });
@@ -368,7 +391,11 @@ export default class WebpackBundler extends Plugin implements Bundler {
368391

369392
// Handling full-name imports that point at the app itself e.g. app-name/lib/thingy
370393
if (name === this.opts.rootPackage.name) {
371-
if (this.importMatchesAppImports(request.slice(name.length + 1))) {
394+
if (
395+
this.importMatchesAppImports(
396+
stripQuery(request.slice(name.length + 1))
397+
)
398+
) {
372399
// webpack should handle this because it's another file in the app that matches allowAppImports
373400
return callback();
374401
} else {
@@ -481,7 +508,7 @@ export default class WebpackBundler extends Plugin implements Bundler {
481508
/EAI_DISCOVERED_EXTERNALS\(['"]([^'"]+)['"]\)/g,
482509
(_substr: string, matched: string) => {
483510
let deps = build
484-
.externalDepsFor(matched)
511+
.externalDepsFor(idToModule(matched))
485512
.filter((dep) => this.externalizedByUs.has(dep));
486513
return '[' + deps.map((d) => `'${d}'`).join(',') + ']';
487514
}

test-scenarios/static-import-test.ts

+46-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,19 @@ function staticImportTest(project: Project) {
4242
'original-package'
4343
],
4444
allowAppImports: [
45-
'lib/**'
46-
]
45+
'lib/**',
46+
'assets/*.specialfile',
47+
],
48+
webpack: {
49+
module: {
50+
rules: [
51+
{
52+
test: /\.specialfile/,
53+
use: 'specialfile-loader',
54+
},
55+
],
56+
},
57+
},
4758
}
4859
});
4960
return app.toTree();
@@ -132,6 +143,9 @@ function staticImportTest(project: Project) {
132143
`,
133144
},
134145
},
146+
assets: {
147+
'test.specialfile': 'This is just plain text.',
148+
},
135149
},
136150
tests: {
137151
helpers: {
@@ -200,6 +214,8 @@ function staticImportTest(project: Project) {
200214
dont_find_me_4,
201215
secret_string_7
202216
} from '../../lib/example2';
217+
import testAsset from '@ef4/app-template/assets/test.specialfile';
218+
import { query as testAssetQuery } from '@ef4/app-template/assets/test.specialfile?foo=bar';
203219
import Service from '@ember/service';
204220
import example6Direct, { dont_find_me } from '@ef4/app-template/utils/example6';
205221
import example7Direct, { secret_string } from '@ef4/app-template/utils/example7';
@@ -280,6 +296,20 @@ function staticImportTest(project: Project) {
280296
'should not have example3 in loader'
281297
);
282298
});
299+
test('it can import assets handled by loader', function (assert) {
300+
assert.strictEqual(
301+
testAsset,
302+
'This is just plain text.',
303+
'Content loaded from customloader can be imported'
304+
);
305+
});
306+
test('it can import assets that have query params', function (assert) {
307+
assert.strictEqual(
308+
testAssetQuery,
309+
'?foo=bar',
310+
'query params are correctly handled'
311+
);
312+
});
283313
});
284314
`,
285315
},
@@ -312,6 +342,20 @@ function staticImportTest(project: Project) {
312342
}`,
313343
},
314344
});
345+
346+
project.addDevDependency('specialfile-loader', {
347+
files: {
348+
'index.js': `
349+
export default function customLoader(source) {
350+
return \`export const query = \${JSON.stringify(this.resourceQuery)};
351+
export default \${JSON.stringify(source)}\`;
352+
}
353+
`,
354+
'package.json': JSON.stringify({
355+
type: 'module',
356+
}),
357+
},
358+
});
315359
}
316360

317361
let scenarios = appScenarios.map('static-import', project => {

0 commit comments

Comments
 (0)