Merge pull request #1846 from embroider-build/handle-vite-query-params

Prevent query-params added by vite to be passed to core logic

Author: ef4

packages/shared-internals/src/index.ts

@@ -1,5 +1,5 @@
 export { AppMeta, AddonMeta, PackageInfo } from './metadata';
-export { explicitRelative, extensionsPattern, unrelativize, cleanUrl } from './paths';
+export { explicitRelative, extensionsPattern, unrelativize, cleanUrl, getUrlQueryParams } from './paths';
 export { getOrCreate } from './get-or-create';
 export { default as Package, V2AddonPackage as AddonPackage, V2AppPackage as AppPackage, V2Package } from './package';
 export { default as PackageCache } from './package-cache';

packages/shared-internals/src/paths.ts

@@ -44,8 +44,22 @@ 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.
-export function cleanUrl(url: string): string {
-  return url.replace(postfixRE, '');
+// includeHashSign true means #my-specifier is considered part of the pathname
+export function cleanUrl(url: string, includeHashSign = false): string {
+  const regexp = includeHashSign ? postfixREQueryParams : 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;
+  return url.match(regexp)?.[0] ?? '';
 }

packages/vite/src/request.ts

@@ -1,5 +1,5 @@
 import type { ModuleRequest, Resolution } from '@embroider/core';
-import { cleanUrl } from '@embroider/core';
+import { cleanUrl, getUrlQueryParams } from '@embroider/core';
 import type { PluginContext, ResolveIdResult } from 'rollup';
 
 export const virtualPrefix = 'embroider_virtual:';
@@ -28,7 +28,24 @@ export class RollupModuleRequest implements ModuleRequest {
 
       // strip query params off the importer
       let fromFile = cleanUrl(nonVirtual);
-      return new RollupModuleRequest(context, source, fromFile, custom?.embroider?.meta, false, undefined);
+      let importerQueryParams = getUrlQueryParams(nonVirtual);
+
+      // 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);
+
+      return new RollupModuleRequest(
+        context,
+        cleanSource,
+        fromFile,
+        custom?.embroider?.meta,
+        false,
+        undefined,
+        queryParams,
+        importerQueryParams
+      );
     }
   }
 
@@ -38,7 +55,9 @@ export class RollupModuleRequest implements ModuleRequest {
     readonly fromFile: string,
     readonly meta: Record<string, any> | undefined,
     readonly isNotFound: boolean,
-    readonly resolvedTo: Resolution<ResolveIdResult> | undefined
+    readonly resolvedTo: Resolution<ResolveIdResult> | undefined,
+    private queryParams: string,
+    private importerQueryParams: string
   ) {}
 
   get debugType() {
@@ -49,14 +68,40 @@ export class RollupModuleRequest implements ModuleRequest {
     return this.specifier.startsWith(virtualPrefix);
   }
 
+  private get specifierWithQueryParams(): string {
+    return `${this.specifier}${this.queryParams}`;
+  }
+
+  private get fromFileWithQueryParams(): string {
+    return `${this.fromFile}${this.importerQueryParams}`;
+  }
+
   alias(newSpecifier: string) {
-    return new RollupModuleRequest(this.context, newSpecifier, this.fromFile, this.meta, false, undefined) as this;
+    return new RollupModuleRequest(
+      this.context,
+      newSpecifier,
+      this.fromFile,
+      this.meta,
+      false,
+      undefined,
+      this.queryParams,
+      this.importerQueryParams
+    ) as this;
   }
   rehome(newFromFile: string) {
     if (this.fromFile === newFromFile) {
       return this;
     } else {
-      return new RollupModuleRequest(this.context, this.specifier, newFromFile, this.meta, false, undefined) as this;
+      return new RollupModuleRequest(
+        this.context,
+        this.specifier,
+        newFromFile,
+        this.meta,
+        false,
+        undefined,
+        this.queryParams,
+        this.importerQueryParams
+      ) as this;
     }
   }
   virtualize(filename: string) {
@@ -66,7 +111,9 @@ export class RollupModuleRequest implements ModuleRequest {
       this.fromFile,
       this.meta,
       false,
-      undefined
+      undefined,
+      this.queryParams,
+      this.importerQueryParams
     ) as this;
   }
   withMeta(meta: Record<string, any> | undefined): this {
@@ -76,29 +123,40 @@ export class RollupModuleRequest implements ModuleRequest {
       this.fromFile,
       meta,
       this.isNotFound,
-      this.resolvedTo
+      this.resolvedTo,
+      this.queryParams,
+      this.importerQueryParams
     ) as this;
   }
   notFound(): this {
-    return new RollupModuleRequest(this.context, this.specifier, this.fromFile, this.meta, true, undefined) as this;
+    return new RollupModuleRequest(
+      this.context,
+      this.specifier,
+      this.fromFile,
+      this.meta,
+      true,
+      undefined,
+      this.queryParams,
+      this.importerQueryParams
+    ) as this;
   }
   async defaultResolve(): Promise<Resolution<ResolveIdResult>> {
     if (this.isVirtual) {
       return {
         type: 'found',
         filename: this.specifier,
-        result: { id: this.specifier, resolvedBy: this.fromFile },
+        result: { id: this.specifierWithQueryParams, resolvedBy: this.fromFileWithQueryParams },
         isVirtual: this.isVirtual,
       };
     }
     if (this.isNotFound) {
       // TODO: we can make sure this looks correct in rollup & vite output when a
       // user encounters it
-      let err = new Error(`module not found ${this.specifier}`);
+      let err = new Error(`module not found ${this.specifierWithQueryParams}`);
       (err as any).code = 'MODULE_NOT_FOUND';
       return { type: 'not_found', err };
     }
-    let result = await this.context.resolve(this.specifier, this.fromFile, {
+    let result = await this.context.resolve(this.specifierWithQueryParams, this.fromFileWithQueryParams, {
       skipSelf: true,
       custom: {
         embroider: {
@@ -108,7 +166,8 @@ export class RollupModuleRequest implements ModuleRequest {
       },
     });
     if (result) {
-      return { type: 'found', filename: result.id, result, isVirtual: this.isVirtual };
+      let { pathname } = new URL(result.id, 'http://example.com');
+      return { type: 'found', filename: pathname, result, isVirtual: this.isVirtual };
     } else {
       return { type: 'not_found', err: undefined };
     }
@@ -121,7 +180,9 @@ export class RollupModuleRequest implements ModuleRequest {
       this.fromFile,
       this.meta,
       this.isNotFound,
-      resolution
+      resolution,
+      this.queryParams,
+      this.importerQueryParams
     ) as this;
   }
 }

packages/vite/src/resolver.ts

@@ -52,7 +52,8 @@ export function resolver(): Plugin {
     },
     load(id) {
       if (id.startsWith(virtualPrefix)) {
-        let { src, watches } = virtualContent(id.slice(virtualPrefix.length), resolverLoader.resolver);
+        let { pathname } = new URL(id, 'http://example.com');
+        let { src, watches } = virtualContent(pathname.slice(virtualPrefix.length + 1), resolverLoader.resolver);
         virtualDeps.set(id, watches);
         server?.watcher.add(watches);
         return src;