From 7b68bdf8d4a8b2d0bfb26ef08b13b91fc3eeb5ae Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 8 Oct 2024 09:54:45 -0400 Subject: [PATCH 1/2] Vite hbs plugin should return resolutions The lack of the return is probably doubling the amount of resoultion work in downstream plugins. Also the extra fallback here should not be needed anymore given that the default resolve is supposed to check for hbs. --- packages/vite/src/hbs.ts | 44 +++++++++------------------------------- 1 file changed, 10 insertions(+), 34 deletions(-) diff --git a/packages/vite/src/hbs.ts b/packages/vite/src/hbs.ts index 2b3acf784..23e85bcb3 100644 --- a/packages/vite/src/hbs.ts +++ b/packages/vite/src/hbs.ts @@ -7,7 +7,6 @@ import { needsSyntheticComponentJS, isInComponents, templateOnlyComponentSource, - syntheticJStoHBS, } from '@embroider/core'; const resolverLoader = new ResolverLoader(process.cwd()); @@ -32,44 +31,21 @@ export function hbs(): Plugin { skipSelf: true, }); - if (!resolution) { - let hbsSource = syntheticJStoHBS(source); - if (hbsSource) { - resolution = await this.resolve(hbsSource, importer, { - skipSelf: true, - custom: { - embroider: { - // we don't want to recurse into the whole embroider compatbility - // resolver here. It has presumably already steered our request to the - // correct place. All we want to do is slightly modify the request we - // were given (changing the extension) and check if that would resolve - // instead. - // - // Currently this guard is only actually exercised in rollup, not in - // vite, due to https://github.com/vitejs/vite/issues/13852 - enableCustomResolver: false, - isExtensionSearch: true, + if (resolution) { + let syntheticId = needsSyntheticComponentJS(source, resolution.id); + if (syntheticId && isInComponents(resolution.id, resolverLoader.resolver.packageCache)) { + return { + id: syntheticId, + meta: { + 'rollup-hbs-plugin': { + type: 'template-only-component-js', }, }, - }); - } - - if (!resolution) { - return null; + }; } } - let syntheticId = needsSyntheticComponentJS(source, resolution.id); - if (syntheticId && isInComponents(resolution.id, resolverLoader.resolver.packageCache)) { - return { - id: syntheticId, - meta: { - 'rollup-hbs-plugin': { - type: 'template-only-component-js', - }, - }, - }; - } + return resolution; }, load(id: string) { From d49730640dc6b464b4fd9c7176d4a559b61d5635 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Tue, 8 Oct 2024 10:18:47 -0400 Subject: [PATCH 2/2] restoring an hbs fallback case that was actually needed --- packages/vite/src/hbs.ts | 47 ++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/packages/vite/src/hbs.ts b/packages/vite/src/hbs.ts index 23e85bcb3..793a09838 100644 --- a/packages/vite/src/hbs.ts +++ b/packages/vite/src/hbs.ts @@ -7,6 +7,7 @@ import { needsSyntheticComponentJS, isInComponents, templateOnlyComponentSource, + syntheticJStoHBS, } from '@embroider/core'; const resolverLoader = new ResolverLoader(process.cwd()); @@ -31,18 +32,46 @@ export function hbs(): Plugin { skipSelf: true, }); - if (resolution) { - let syntheticId = needsSyntheticComponentJS(source, resolution.id); - if (syntheticId && isInComponents(resolution.id, resolverLoader.resolver.packageCache)) { - return { - id: syntheticId, - meta: { - 'rollup-hbs-plugin': { - type: 'template-only-component-js', + if (!resolution) { + // vite already has extension search fallback for extensionless imports. + // This is different, it covers an explicit .js import fallback to the + // corresponding hbs. + let hbsSource = syntheticJStoHBS(source); + if (hbsSource) { + resolution = await this.resolve(hbsSource, importer, { + skipSelf: true, + custom: { + embroider: { + // we don't want to recurse into the whole embroider compatbility + // resolver here. It has presumably already steered our request to the + // correct place. All we want to do is slightly modify the request we + // were given (changing the extension) and check if that would resolve + // instead. + // + // Currently this guard is only actually exercised in rollup, not in + // vite, due to https://github.com/vitejs/vite/issues/13852 + enableCustomResolver: false, + isExtensionSearch: true, }, }, - }; + }); } + + if (!resolution) { + return null; + } + } + + let syntheticId = needsSyntheticComponentJS(source, resolution.id); + if (syntheticId && isInComponents(resolution.id, resolverLoader.resolver.packageCache)) { + return { + id: syntheticId, + meta: { + 'rollup-hbs-plugin': { + type: 'template-only-component-js', + }, + }, + }; } return resolution;