From d14ae9afbf158bbdb54b73e3901a5b765dbcc29c Mon Sep 17 00:00:00 2001 From: Simon Ihmig Date: Sun, 3 Dec 2023 20:33:13 +0100 Subject: [PATCH] Fix imports with a query part With the new `allowAppImports` feature, apps can import resources that are handledwith custom webpack loaders. Some can allow you to customize their behavior by passing query params, like `import image from './images/image.jpg?size=1024`. This PR is fixing two related issues: * make sure that an `allowAppImports` glob like `'images/**/*.jpg'` would also match for imports that include query params, by stripping the query part before trying to match the import to the glob * query params could technically also include quotes. These already got escaped when building the app-chunk that has the AMD-shims, but the regex to replace `EAI_DISCOVERED_EXTERNALS` was not taking their possible existance into account correctly. --- packages/ember-auto-import/ts/package.ts | 5 +++- packages/ember-auto-import/ts/util.ts | 4 +++ packages/ember-auto-import/ts/webpack.ts | 35 +++++++++++++++++++++--- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/packages/ember-auto-import/ts/package.ts b/packages/ember-auto-import/ts/package.ts index dd9ea3bb9..f4c9f51a9 100644 --- a/packages/ember-auto-import/ts/package.ts +++ b/packages/ember-auto-import/ts/package.ts @@ -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 = new WeakMap(); @@ -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', diff --git a/packages/ember-auto-import/ts/util.ts b/packages/ember-auto-import/ts/util.ts index 540f45f62..6859aca2f 100644 --- a/packages/ember-auto-import/ts/util.ts +++ b/packages/ember-auto-import/ts/util.ts @@ -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]; +} diff --git a/packages/ember-auto-import/ts/webpack.ts b/packages/ember-auto-import/ts/webpack.ts index 75709f579..714f6fe0f 100644 --- a/packages/ember-auto-import/ts/webpack.ts +++ b/packages/ember-auto-import/ts/webpack.ts @@ -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'; @@ -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']; @@ -35,6 +37,27 @@ registerHelper('join', function (list, connector) { return list.join(connector); }); +const moduleIdMap = new Map(); + +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(){ @@ -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}}'); }); @@ -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 { @@ -478,10 +505,10 @@ export default class WebpackBundler extends Plugin implements Bundler { 'utf8' ); let outputSrc = inputSrc.replace( - /EAI_DISCOVERED_EXTERNALS\(['"]([^'"]+)['"]\)/g, + /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(',') + ']'; }