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

make addon appjs virtual #2109

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
084183d
make appjs virtual
patricklx Sep 13, 2024
245a2ef
fix
patricklx Sep 14, 2024
9390ae7
fix
patricklx Sep 14, 2024
e3047c7
fix
patricklx Sep 14, 2024
d83c1c7
change order
patricklx Sep 14, 2024
d2fd327
fix
patricklx Sep 14, 2024
dcd8752
fix
patricklx Sep 14, 2024
a9a86a7
fix virtual app file
patricklx Sep 14, 2024
c173c33
is this needed?
patricklx Sep 14, 2024
4904294
remove
patricklx Sep 14, 2024
543298b
fix
patricklx Sep 14, 2024
beac07c
revert
patricklx Sep 14, 2024
2658927
fix
patricklx Sep 14, 2024
b61817f
fix
patricklx Sep 14, 2024
da65c7a
fix
patricklx Sep 14, 2024
c0ed7f3
app js is always resolvable from root
patricklx Sep 14, 2024
03ca207
fix
patricklx Sep 14, 2024
14c4d10
fix
patricklx Sep 14, 2024
81793d1
fix
patricklx Sep 14, 2024
ce008bf
fix
patricklx Sep 14, 2024
8d1255e
revert
patricklx Sep 14, 2024
102cc28
fix virtual
patricklx Sep 15, 2024
3881287
full path
patricklx Sep 15, 2024
92cc281
revert
patricklx Sep 15, 2024
a605ed3
fix
patricklx Sep 15, 2024
578836b
fix
patricklx Sep 15, 2024
3a8e930
fix
patricklx Sep 15, 2024
d66f43a
fix
patricklx Sep 15, 2024
44ece7f
fix
patricklx Sep 15, 2024
1940b8e
fix
patricklx Sep 15, 2024
ab7356e
fix
patricklx Sep 15, 2024
684e113
revert
patricklx Sep 15, 2024
8e2ec16
change name
patricklx Sep 15, 2024
a012b7a
fix
patricklx Sep 15, 2024
43b3c03
fix
patricklx Sep 15, 2024
cbe043f
fix
patricklx Sep 15, 2024
a43dac7
logs
patricklx Sep 15, 2024
f3e0b67
fix
patricklx Sep 16, 2024
8a1c519
fix core tests
patricklx Sep 16, 2024
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
13 changes: 13 additions & 0 deletions packages/compat/src/babel-plugin-adjust-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@ export default function main(babel: typeof Babel) {
}

return {
manipulateOptions(opts: any, parserOpts: any) {
let filename: string = opts.filename || parserOpts.sourceFileName;
if (!filename) return;
filename = cleanUrl(filename);
if (filename.includes('__embroider_appjs_match__')) {
filename = filename.split('__embroider_appjs_match__')[0];
if (filename.includes('embroider_virtual:')) {
filename = filename.split('embroider_virtual:')[1];
}
opts.filename = filename;
parserOpts.sourceFileName = filename;
}
},
visitor: {
Program: {
enter(path: NodePath<t.Program>, state: State) {
Expand Down
1 change: 1 addition & 0 deletions packages/compat/src/module-visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ class ModuleVisitor {
transpiledContent: result.transpiledContent,
};
} catch (err) {
console.error(err);
if (['BABEL_PARSE_ERROR', 'BABEL_TRANSFORM_ERROR'].includes(err.code)) {
return [
{
Expand Down
37 changes: 34 additions & 3 deletions packages/core/src/module-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
fastbootSwitch,
decodeFastbootSwitch,
decodeImplicitModules,
decodeAppJsMatch,
encodeAppJsMatch,
} from './virtual-content';
import { Memoize } from 'typescript-memoize';
import { describeExports } from './describe-exports';
Expand Down Expand Up @@ -168,6 +170,7 @@ export class Resolver {

request = this.handleFastbootSwitch(request);
request = await this.handleGlobalsCompat(request);
request = this.handleEncodedAppJsMatch(request);
request = this.handleImplicitModules(request);
request = this.handleImplicitTestScripts(request);
request = this.handleVendorStyles(request);
Expand All @@ -184,6 +187,7 @@ export class Resolver {
// rehome requests to their un-rewritten locations, and for the most part we
// want to be dealing with the rewritten packages.
request = this.handleRewrittenPackages(request);
request = this.handleAppJsMatch(request);
return request;
}

Expand Down Expand Up @@ -372,6 +376,27 @@ export class Resolver {
return logTransition('failed to match in fastboot switch', request);
}

private handleAppJsMatch<R extends ModuleRequest>(request: R): R {
if (request.meta?.isAppJsMatch) {
return request.virtualize(encodeAppJsMatch(request.specifier, request.fromFile));
}
return request;
}

private handleEncodedAppJsMatch<R extends ModuleRequest>(request: R): R {
if (isTerminal(request)) {
return request;
}
let match = decodeAppJsMatch(request.fromFile);
if (!match) {
return request;
}
if (match.virtual) {
return request.rehome('./package.json');
}
return request.rehome(match.filename);
}

private handleImplicitModules<R extends ModuleRequest>(request: R): R {
if (isTerminal(request)) {
return request;
Expand Down Expand Up @@ -919,7 +944,7 @@ export class Resolver {
request
// setting meta here because if this fails, we want the fallback
// logic to revert our rehome and continue from the *moved* package.
.withMeta({ originalFromFile: request.fromFile })
.withMeta({ originalFromFile: request.fromFile, isAppJsMatch: request.meta?.isAppJsMatch })
.rehome(resolve(originalRequestingPkg.root, 'package.json'))
);
} else {
Expand Down Expand Up @@ -1055,7 +1080,7 @@ export class Resolver {

// setting meta because if this fails, we want the fallback to pick up back
// in the original requesting package.
return newRequest.withMeta({ originalFromFile });
return newRequest.withMeta({ originalFromFile, isAppJsMatch: request.meta?.isAppJsMatch });
}

private preHandleExternal<R extends ModuleRequest>(request: R): R {
Expand Down Expand Up @@ -1425,7 +1450,13 @@ export class Resolver {
case undefined:
return undefined;
case 'app-only':
return request.alias(matched.entry['app-js'].specifier).rehome(matched.entry['app-js'].fromFile);
return request
.alias(matched.entry['app-js'].specifier)
.rehome(matched.entry['app-js'].fromFile)
.withMeta({
...request.meta,
isAppJsMatch: true,
});
case 'fastboot-only':
return request.alias(matched.entry['fastboot-js'].specifier).rehome(matched.entry['fastboot-js'].fromFile);
case 'both':
Expand Down
62 changes: 57 additions & 5 deletions packages/core/src/virtual-content.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { dirname, basename, resolve, posix, sep, join } from 'path';
import type { Resolver, AddonPackage, Package } from '.';
import { basename, dirname, join, posix, resolve, sep, extname } from 'path';
import type { AddonPackage, Package, Resolver } from '.';
import { explicitRelative, extensionsPattern } from '.';
import { compile } from './js-handlebars';
import { decodeImplicitTestScripts, renderImplicitTestScripts } from './virtual-test-support';
Expand All @@ -9,6 +9,7 @@ import { decodeVirtualVendorStyles, renderVendorStyles } from './virtual-vendor-

import { decodeEntrypoint, renderEntrypoint } from './virtual-entrypoint';
import { decodeRouteEntrypoint, renderRouteEntrypoint } from './virtual-route-entrypoint';
import { readFileSync } from 'fs-extra';

const externalESPrefix = '/@embroider/ext-es/';
const externalCJSPrefix = '/@embroider/ext-cjs/';
Expand Down Expand Up @@ -42,6 +43,15 @@ export function virtualContent(filename: string, resolver: Resolver): VirtualCon
if (extern) {
return renderESExternalShim(extern);
}

let appjs = decodeAppJsMatch(filename);
if (appjs) {
if (!appjs.virtual) {
return renderAppJs(appjs.filename);
}
filename = appjs.filename;
}

let match = decodeVirtualPairComponent(filename);
if (match) {
return pairedComponentShim(match);
Expand Down Expand Up @@ -175,8 +185,8 @@ const pairComponentMarker = '-embroider-pair-component';
const pairComponentPattern = /^(?<hbsModule>.*)__vpc__(?<jsModule>[^\/]*)-embroider-pair-component$/;

export function virtualPairComponent(hbsModule: string, jsModule: string | undefined): string {
let relativeJSModule = '';
if (jsModule) {
let relativeJSModule = jsModule || '';
if (jsModule && !jsModule.includes(appJsMatchMarker)) {
relativeJSModule = explicitRelative(dirname(hbsModule), jsModule);
}
return `${hbsModule}__vpc__${encodeURIComponent(relativeJSModule)}${pairComponentMarker}`;
Expand All @@ -195,14 +205,56 @@ function decodeVirtualPairComponent(
}
let { hbsModule, jsModule } = match.groups! as { hbsModule: string; jsModule: string };
// target our real hbs module from our virtual module
let relativeHBSModule = explicitRelative(dirname(filename), hbsModule);
let relativeHBSModule = hbsModule;
if (!hbsModule.includes(appJsMatchMarker)) {
relativeHBSModule = explicitRelative(dirname(filename), hbsModule);
}
return {
relativeHBSModule,
relativeJSModule: decodeURIComponent(jsModule) || null,
debugName: basename(relativeHBSModule).replace(/\.(js|hbs)$/, ''),
};
}

const appJsMatchMarker = '__embroider_appjs_match__';
const appJsMatchPattern = /(?<to>.+)__embroider_appjs_match__.{2,5}$/;
export function encodeAppJsMatch(specifier: string, from: string): string {
let to = require.resolve(specifier, {
paths: [resolve(dirname(from), 'node_modules')],
});
return `${to}${appJsMatchMarker}${extname(to)}`;
}

export function decodeAppJsMatch(filename: string) {
// Performance: avoid paying regex exec cost unless needed
if (!filename.includes(appJsMatchMarker)) {
return;
}
let match = appJsMatchPattern.exec(filename);
if (match) {
let to = match.groups!.to;
if (to.includes(pairComponentMarker)) {
if (to.includes('_vpc_')) {
to = filename;
}
return {
filename: to,
virtual: true,
};
}
return {
filename: to,
};
}
}

function renderAppJs(filename: string) {
return {
src: readFileSync(filename).toString(),
watches: [filename],
};
}

const fastbootSwitchSuffix = '/embroider_fastboot_switch';
const fastbootSwitchPattern = /(?<original>.+)\/embroider_fastboot_switch(?:\?names=(?<names>.+))?$/;
export function fastbootSwitch(specifier: string, fromFile: string, names: Set<string>): string {
Expand Down
14 changes: 7 additions & 7 deletions tests/scenarios/core-resolver-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ Scenarios.fromProject(() => new Project())
expectAudit
.module('./app.js')
.resolves('my-app/hello-world')
.to('./node_modules/my-addon/_app_/hello-world.js');
.to('./node_modules/my-addon/_app_/hello-world.js__embroider_appjs_match__.js');
});

test('app-js module in addon can still do relative imports that escape its package', async function () {
Expand All @@ -527,7 +527,7 @@ Scenarios.fromProject(() => new Project())
});

expectAudit
.module('./node_modules/my-addon/_app_/hello-world.js')
.module('./node_modules/my-addon/_app_/hello-world.js__embroider_appjs_match__.js')
.resolves('../../extra.js')
.to('./node_modules/extra.js');
});
Expand All @@ -547,7 +547,7 @@ Scenarios.fromProject(() => new Project())
expectAudit
.module('./app.js')
.resolves('my-app/templates/hello-world')
.to('./node_modules/my-addon/_app_/templates/hello-world.hbs');
.to('./node_modules/my-addon/_app_/templates/hello-world.hbs__embroider_appjs_match__.hbs');
});

test(`relative import in addon's app tree resolves to app`, async function () {
Expand All @@ -564,7 +564,7 @@ Scenarios.fromProject(() => new Project())
});

expectAudit
.module('./node_modules/my-addon/_app_/hello-world.js')
.module('./node_modules/my-addon/_app_/hello-world.js__embroider_appjs_match__.js')
.resolves('./secondary')
.to('./secondary.js');
});
Expand All @@ -584,7 +584,7 @@ Scenarios.fromProject(() => new Project())
});

expectAudit
.module('./node_modules/my-addon/_app_/hello-world.js')
.module('./node_modules/my-addon/_app_/hello-world.js__embroider_appjs_match__.js')
.resolves('./secondary')
.to('./secondary.js');
});
Expand All @@ -602,7 +602,7 @@ Scenarios.fromProject(() => new Project())
});

expectAudit
.module('./node_modules/my-addon/_app_/hello-world.js')
.module('./node_modules/my-addon/_app_/hello-world.js__embroider_appjs_match__.js')
.resolves('the-apps-dep')
.to('./node_modules/the-apps-dep/index.js');
});
Expand All @@ -621,7 +621,7 @@ Scenarios.fromProject(() => new Project())
});

expectAudit
.module('./node_modules/my-addon/_app_/hello-world.js')
.module('./node_modules/my-addon/_app_/hello-world.js__embroider_appjs_match__.js')
.resolves('my-app/secondary')
.to('./secondary.js');
});
Expand Down
27 changes: 20 additions & 7 deletions tests/scenarios/vite-internals-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,20 +219,33 @@ appScenarios

test(`dep optimization of a v2 addon`, async function (assert) {
expectAudit
.module('./index.html')
.resolves(/\/index.html.*/) // in-html app-boot script
.toModule()
.resolves(/\/app\.js.*/)
.module(/\/app\.js.*/)
.resolves(/.*\/-embroider-entrypoint.js/)
.toModule()
.withContents((_src, imports) => {
let pageTitleImports = imports.filter(imp => /page-title/.test(imp.source));
assert.ok(pageTitleImports.length > 0, 'should not have at least one import from page-title');
for (let pageTitleImport of pageTitleImports) {
assert.notOk(
/\.vite\/deps/.test(pageTitleImport.source),
`expected ${pageTitleImport.source} not to be in vite deps`
);
}
return true;
});
expectAudit
.module(/\/app\.js.*/)
.resolves(/.*\/-embroider-entrypoint.js/)
.toModule()
.resolves(/page-title/)
.toModule()
.withContents((_src, imports) => {
let pageTitleImports = imports.filter(imp => /page-title/.test(imp.source));
assert.ok(pageTitleImports.length > 0, 'should have at least one import from page-title');
assert.ok(pageTitleImports.length > 0, 'should not have at least one import from page-title');
for (let pageTitleImport of pageTitleImports) {
assert.ok(
assert.notOk(
/\.vite\/deps/.test(pageTitleImport.source),
`expected ${pageTitleImport.source} to be in vite deps`
`expected ${pageTitleImport.source} not to be in vite deps`
);
}
return true;
Expand Down
Loading