Skip to content

Commit

Permalink
Merge pull request #2028 from embroider-build/rename-hashtag-compat
Browse files Browse the repository at this point in the history
Rename #embroider_compat/* to @embroider/virtual/*
  • Loading branch information
mansona authored Jul 29, 2024
2 parents 024270b + 7a4d2cb commit 9372659
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 130 deletions.
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

0 comments on commit 9372659

Please sign in to comment.