Skip to content

Commit

Permalink
Merge pull request #603 from embroider-build/fix-imports-with-query
Browse files Browse the repository at this point in the history
Fix imports with a query part
  • Loading branch information
ef4 authored Dec 12, 2023
2 parents 8e2c9a8 + 12e5c09 commit 5204edf
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 6 deletions.
5 changes: 4 additions & 1 deletion packages/ember-auto-import/ts/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import semver from 'semver';
import type { TransformOptions } from '@babel/core';
import { MacrosConfig } from '@embroider/macros/src/node';
import minimatch from 'minimatch';
import { stripQuery } from './util';

// from child addon instance to their parent package
const parentCache: WeakMap<AddonInstance, Package> = new WeakMap();
Expand Down Expand Up @@ -315,7 +316,9 @@ export default class Package {
if (!this.isAddon && packageName === this.name) {
let localPath = path.slice(packageName.length + 1);
if (
this.allowAppImports.some((pattern) => minimatch(localPath, pattern))
this.allowAppImports.some((pattern) =>
minimatch(stripQuery(localPath), pattern)
)
) {
return {
type: 'package',
Expand Down
4 changes: 4 additions & 0 deletions packages/ember-auto-import/ts/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ export function shallowEqual(a: any[], b: any[]) {
a.every((item, index) => item === b[index])
);
}

export function stripQuery(path: string) {
return path.split('?')[0];
}
33 changes: 30 additions & 3 deletions packages/ember-auto-import/ts/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
Module,
} from 'webpack';
import { join, dirname, resolve, relative, posix, isAbsolute } from 'path';
import { createHash } from 'crypto';
import { mergeWith, flatten, zip } from 'lodash';
import { writeFileSync, realpathSync, readFileSync } from 'fs';
import { compile, registerHelper } from 'handlebars';
Expand All @@ -25,6 +26,7 @@ import { ensureDirSync, symlinkSync, existsSync } from 'fs-extra';
import MiniCssExtractPlugin from 'mini-css-extract-plugin';
import minimatch from 'minimatch';
import { TransformOptions } from '@babel/core';
import { stripQuery } from './util';

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

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

const moduleIdMap = new Map<string, string>();

function moduleToId(moduleSpecifier: string) {
let id = moduleSpecifier;

// if the module contains characters that need to be escaped, then map this to a hash instead, so we can easily replace this later
if (moduleSpecifier.includes('"') || moduleSpecifier.includes("'")) {
id = createHash('md5').update('some_string').digest('hex');

moduleIdMap.set(id, moduleSpecifier);
}

return id;
}

function idToModule(id: string) {
return moduleIdMap.get(id) ?? id;
}

registerHelper('module-to-id', moduleToId);

const entryTemplate = compile(
`
module.exports = (function(){
Expand All @@ -52,7 +75,7 @@ module.exports = (function(){
return r('_eai_sync_' + specifier)(Array.prototype.slice.call(arguments, 1))
};
{{#each staticImports as |module|}}
d('{{js-string-escape module.specifier}}', EAI_DISCOVERED_EXTERNALS('{{js-string-escape module.specifier}}'), function() { return require('{{js-string-escape module.specifier}}'); });
d('{{js-string-escape module.specifier}}', EAI_DISCOVERED_EXTERNALS('{{module-to-id module.specifier}}'), function() { return require('{{js-string-escape module.specifier}}'); });
{{/each}}
{{#each dynamicImports as |module|}}
d('_eai_dyn_{{js-string-escape module.specifier}}', [], function() { return import('{{js-string-escape module.specifier}}'); });
Expand Down Expand Up @@ -368,7 +391,11 @@ export default class WebpackBundler extends Plugin implements Bundler {

// Handling full-name imports that point at the app itself e.g. app-name/lib/thingy
if (name === this.opts.rootPackage.name) {
if (this.importMatchesAppImports(request.slice(name.length + 1))) {
if (
this.importMatchesAppImports(
stripQuery(request.slice(name.length + 1))
)
) {
// webpack should handle this because it's another file in the app that matches allowAppImports
return callback();
} else {
Expand Down Expand Up @@ -481,7 +508,7 @@ export default class WebpackBundler extends Plugin implements Bundler {
/EAI_DISCOVERED_EXTERNALS\(['"]([^'"]+)['"]\)/g,
(_substr: string, matched: string) => {
let deps = build
.externalDepsFor(matched)
.externalDepsFor(idToModule(matched))
.filter((dep) => this.externalizedByUs.has(dep));
return '[' + deps.map((d) => `'${d}'`).join(',') + ']';
}
Expand Down
48 changes: 46 additions & 2 deletions test-scenarios/static-import-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,19 @@ function staticImportTest(project: Project) {
'original-package'
],
allowAppImports: [
'lib/**'
]
'lib/**',
'assets/*.specialfile',
],
webpack: {
module: {
rules: [
{
test: /\.specialfile/,
use: 'specialfile-loader',
},
],
},
},
}
});
return app.toTree();
Expand Down Expand Up @@ -132,6 +143,9 @@ function staticImportTest(project: Project) {
`,
},
},
assets: {
'test.specialfile': 'This is just plain text.',
},
},
tests: {
helpers: {
Expand Down Expand Up @@ -200,6 +214,8 @@ function staticImportTest(project: Project) {
dont_find_me_4,
secret_string_7
} from '../../lib/example2';
import testAsset from '@ef4/app-template/assets/test.specialfile';
import { query as testAssetQuery } from '@ef4/app-template/assets/test.specialfile?foo=bar';
import Service from '@ember/service';
import example6Direct, { dont_find_me } from '@ef4/app-template/utils/example6';
import example7Direct, { secret_string } from '@ef4/app-template/utils/example7';
Expand Down Expand Up @@ -280,6 +296,20 @@ function staticImportTest(project: Project) {
'should not have example3 in loader'
);
});
test('it can import assets handled by loader', function (assert) {
assert.strictEqual(
testAsset,
'This is just plain text.',
'Content loaded from customloader can be imported'
);
});
test('it can import assets that have query params', function (assert) {
assert.strictEqual(
testAssetQuery,
'?foo=bar',
'query params are correctly handled'
);
});
});
`,
},
Expand Down Expand Up @@ -312,6 +342,20 @@ function staticImportTest(project: Project) {
}`,
},
});

project.addDevDependency('specialfile-loader', {
files: {
'index.js': `
export default function customLoader(source) {
return \`export const query = \${JSON.stringify(this.resourceQuery)};
export default \${JSON.stringify(source)}\`;
}
`,
'package.json': JSON.stringify({
type: 'module',
}),
},
});
}

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

0 comments on commit 5204edf

Please sign in to comment.