perf(resolve): avoid isWorkerRequest and clean up .ts imported a .js

[patak-dev]

Description

Alternative to #10273

isWorkerRequest is costly, and it was showing in the profiles. There is an internal call to parseRequest, a function we should also review because it uses a regex that for this particular case wasn't needed for example (it is useful with dynamic variables in imports)

Using cleanUrl should work here. The original issue of having .ts?query is more general than just in workers.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[bluwy]

Good catch! Maybe we can simplify the isTsRequest and isPossibleTsOutput functions directly here:

vite/packages/vite/src/node/utils.ts

Lines 285 to 289 in 02a46d7

	const knownTsRE = /\.(?:ts|mts|cts|tsx)$/
	const knownTsOutputRE = /\.(?:js|mjs|cjs|jsx)$/
	export const isTsRequest = (url: string): boolean => knownTsRE.test(url)
	export const isPossibleTsOutput = (url: string): boolean =>
	knownTsOutputRE.test(cleanUrl(url))

So we only have to apply one regex to test it.

[patak-dev]

getPotentialSrcPaths was returning the following:

• .js -> [.ts, .tsx]
• .mjs -> [.mts, .mtsx] 👀
• .cjs -> [.cts, .ctsx] 👁️ 👁️
• .jsx -> [.tsx]

It was also doing a complex regex taking into account query params. This function was hiding the real checks, better to be explicit and have them expanded here.
Now we no longer check for mtsx and ctsx

[patak-dev]

Moved this function here because it isn't a widely used util and it only works without query, that is what we need in tryCleanFsResolve. I think we are pushing too many functions to utils lately and we will be better of delaying moving them there to avoid creating abstractions that aren't really needed

[patak-dev]

We are testing these in the resolve playground

[bluwy]

Nice cleanup 🚀

[AriPerkkio]

FYI This perf PR also fixes a bug we run into in Vitest. I was just about to set up a fix PR for it but this exact line fixed it - thanks! My changes for this regexp would have been identical with this one.
Here's minimal repro for it just in case you want to set a test case to prevent this from breaking in future. The file.ts?query is not detected as TS query and fails to load file.mts.

import { writeFileSync } from "fs";
import { fileURLToPath } from "url";
import { createServer } from "vite";

writeFileSync(
  "./source.ts",
  `import { hello } from "./target.mjs"; // Change this to ".mts" or omit extension and it works too
  hello();`,
  "utf8"
);
writeFileSync(
  "./target.mts",
  `export const hello = () => "Hello from .mts";`,
  "utf8"
);

const __dirname = fileURLToPath(new URL(".", import.meta.url));

const server = await createServer({
  configFile: false,
  root: __dirname,
  server: {
    port: 1337,
  },
});
await server.listen();

const first = await server.transformRequest("./source.ts");
console.log("First worked!", first.code);

await new Promise((resolve) => setTimeout(resolve, 2000));

const second = await server
  .transformRequest("./source.ts?v=123")
  .then((result) => console.log("Second worked!", result.code))
  .catch((e) => console.error("Second failed", e));

await server.close();
process.exit();

[patak-dev]

Awesome! Thanks for letting us know! Maybe not a bad idea to send a PR to add a case for this to one of our playgrounds if you would like.