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

Rename #embroider_compat/* to @embroider/virtual/* #2028

Merged
merged 2 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/compat/src/babel-plugin-adjust-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ function applyRules(
t,
adder,
path,
`#embroider_compat/components/${dasherizedName}`,
`@embroider/virtual/components/${dasherizedName}`,
`${lookup.owningEngine.packageName}/components/${dasherizedName}`
)
);
Expand Down
10 changes: 5 additions & 5 deletions packages/compat/src/resolver-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ class TemplateResolver implements ASTPlugin {
private findRules(absPath: string): PreprocessedComponentRule | undefined {
// when babel is invoked by vite our filenames can have query params still
// hanging off them. That would break rule matching.
absPath = cleanUrl(absPath, true);
absPath = cleanUrl(absPath);

let fileRules = this.rules.files.get(absPath);
let componentRules: PreprocessedComponentRule | undefined;
Expand Down Expand Up @@ -440,7 +440,7 @@ class TemplateResolver implements ASTPlugin {
let componentRules = this.rules.components.get(name);
return {
type: 'component',
specifier: `#embroider_compat/components/${name}`,
specifier: `@embroider/virtual/components/${name}`,
importedName: 'default',
yieldsComponents: componentRules ? componentRules.yieldsSafeComponents : [],
yieldsArguments: componentRules ? componentRules.yieldsArguments : [],
Expand Down Expand Up @@ -516,7 +516,7 @@ class TemplateResolver implements ASTPlugin {

return {
type: 'helper',
specifier: `#embroider_compat/helpers/${path}`,
specifier: `@embroider/virtual/helpers/${path}`,
importedName: 'default',
nameHint: this.nameHint(path),
};
Expand Down Expand Up @@ -658,7 +658,7 @@ class TemplateResolver implements ASTPlugin {
let componentRules = this.rules.components.get(path);
return {
type: 'component',
specifier: `#embroider_compat/ambiguous/${path}`,
specifier: `@embroider/virtual/ambiguous/${path}`,
importedName: 'default',
yieldsComponents: componentRules ? componentRules.yieldsSafeComponents : [],
yieldsArguments: componentRules ? componentRules.yieldsArguments : [],
Expand Down Expand Up @@ -689,7 +689,7 @@ class TemplateResolver implements ASTPlugin {

return {
type: 'modifier',
specifier: `#embroider_compat/modifiers/${path}`,
specifier: `@embroider/virtual/modifiers/${path}`,
importedName: 'default',
nameHint: this.nameHint(path),
};
Expand Down
6 changes: 3 additions & 3 deletions packages/compat/tests/audit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ describe('audit', function () {
expect(withoutCodeFrames(result.findings)).toEqual([
{
message: 'unable to resolve dependency',
detail: '#embroider_compat/components/no-such-thing',
detail: '@embroider/virtual/components/no-such-thing',
filename: './hello.hbs',
},
]);
Expand All @@ -356,7 +356,7 @@ describe('audit', function () {
expect(withoutCodeFrames(result.findings)).toEqual([
{
message: 'unable to resolve dependency',
detail: '#embroider_compat/components/no-such-thing',
detail: '@embroider/virtual/components/no-such-thing',
filename: './app.js',
},
]);
Expand All @@ -375,7 +375,7 @@ describe('audit', function () {
expect(withoutCodeFrames(result.findings)).toEqual([
{
message: 'unable to resolve dependency',
detail: '#embroider_compat/components/no-such-thing',
detail: '@embroider/virtual/components/no-such-thing',
filename: './hello.hbs',
},
]);
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/module-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ type MergeEntry =

type MergeMap = Map</* engine root dir */ string, Map</* withinEngineModuleName */ string, MergeEntry>>;

const compatPattern = /#embroider_compat\/(?<type>[^\/]+)\/(?<rest>.*)/;
const compatPattern = /@embroider\/virtual\/(?<type>[^\/]+)\/(?<rest>.*)/;

export interface ModuleRequest<Res extends Resolution = Resolution> {
readonly specifier: string;
Expand Down Expand Up @@ -594,7 +594,7 @@ export class Resolver {
case 'ambiguous':
return this.resolveHelperOrComponent(rest, engine, request);
default:
throw new Error(`bug: unexepected #embroider_compat specifier: ${request.specifier}`);
throw new Error(`bug: unexepected @embroider/virtual specifier: ${request.specifier}`);
}
}

Expand Down Expand Up @@ -1277,14 +1277,14 @@ export class Resolver {

if (compatPattern.test(request.specifier)) {
// Some kinds of compat requests get rewritten into other things
// deterministically. For example, "#embroider_compat/helpers/whatever"
// deterministically. For example, "@embroider/virtual/helpers/whatever"
// means only "the-current-engine/helpers/whatever", and if that doesn't
// actually exist it's that path that will show up as a missing import.
//
// But others have an ambiguous meaning. For example,
// #embroider_compat/components/whatever can mean several different
// @embroider/virtual/components/whatever can mean several different
// things. If we're unable to find any of them, the actual
// "#embroider_compat" request will fall through all the way to here.
// "@embroider/virtual" request will fall through all the way to here.
//
// In that case, we don't want to externalize that failure. We know it's
// not a classic runtime thing. It's better to let it be a build error
Expand Down
4 changes: 2 additions & 2 deletions packages/shared-internals/src/colocation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export function syntheticJStoHBS(source: string): string | null {
// explicit js is the only case we care about here. Synthetic template JS is
// only ever JS (never TS or anything else). And extensionless imports are
// handled by the default resolving system doing extension search.
if (cleanUrl(source, true).endsWith('.js')) {
if (cleanUrl(source).endsWith('.js')) {
return source.replace(/.js(\?.*)?/, '.hbs$1');
}

Expand All @@ -19,7 +19,7 @@ export function needsSyntheticComponentJS(
foundFile: string,
packageCache: Pick<PackageCache, 'ownerOfFile'>
): string | null {
requestedSpecifier = cleanUrl(requestedSpecifier, true);
requestedSpecifier = cleanUrl(requestedSpecifier);
foundFile = cleanUrl(foundFile);
if (
discoveredImplicitHBS(requestedSpecifier, foundFile) &&
Expand Down
14 changes: 4 additions & 10 deletions packages/shared-internals/src/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,17 @@ export function unrelativize(pkg: Package, specifier: string, fromFile: string)

const postfixRE = /[?#].*$/s;

// this pattern includes URL query params (ex: ?direct)
// but excludes specifiers starting with # (ex: #embroider_compats/components/fancy)
// so when using this pattern, #embroider_compat/fancy would be consider a pathname
// without any params.
const postfixREQueryParams = /[?].*$/s;

// this is the same implementation Vite uses internally to keep its
// cache-busting query params from leaking where they shouldn't.
// includeHashSign true means #my-specifier is considered part of the pathname
export function cleanUrl(url: string, includeHashSign = false): string {
const regexp = includeHashSign ? postfixREQueryParams : postfixRE;
export function cleanUrl(url: string): string {
const regexp = postfixRE;
return url.replace(regexp, '');
}

// includeHashSign true means #my-specifier is considered part of the pathname
export function getUrlQueryParams(url: string, includeHashSign = false): string {
const regexp = includeHashSign ? postfixREQueryParams : postfixRE;
export function getUrlQueryParams(url: string): string {
const regexp = postfixRE;
return url.match(regexp)?.[0] ?? '';
}

Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ export class RollupModuleRequest implements ModuleRequest {
// strip query params off the source but keep track of them
// we use regexp-based methods over a URL object because the
// source can be a relative path.
let cleanSource = cleanUrl(source, true);
let queryParams = getUrlQueryParams(source, true);
let cleanSource = cleanUrl(source);
let queryParams = getUrlQueryParams(source);

return new RollupModuleRequest(
context,
Expand Down
Loading
Loading