Builds are non-deterministic if multiple files share the same

[Rich-Harris]

An edge case to be sure, but one I just encountered: if you have two files like foo.js and foo.ts, snowpack build will not only transform them in random order, it will actually combine the resulting source code into something syntactically nonsensical: https://github.com/Rich-Harris/snowpack-non-deterministic-builds-repro

More realistically, it's possible to imagine that someone might mount two directories to the same path...

{
  mount: {
    a: '/x',
    b: '/x'
  }
}

...in which case collisions are handled deterministically (as far as I can tell), but it might be helpful to be notified of the collision.

[ZYSzys]

I think this is indeed a bug.

One idea to fix this was check whether finalDestLoc already used in 0. Find all source files. phase before building them.

   // 0. Find all source files.
   for (const [mountedDir, mountEntry] of Object.entries(config.mount)) {
+    const finalDestLocSet = new Set();
     const allFiles = glob.sync(`**/*`, {
       ignore: [...config.exclude, ...config.testOptions.files],
       cwd: mountedDir,
@@ -424,6 +425,13 @@ export async function command(commandOptions: CommandOptions) {
       const fileLoc = path.resolve(rawLocOnDisk); // this is necessary since glob.sync() returns paths with / on windows.  path.resolve() will switch them to the native path separator.
       const finalUrl = getUrlForFileMount({fileLoc, mountKey: mountedDir, mountEntry, config})!;
       const finalDestLoc = path.join(buildDirectoryLoc, finalUrl);
+
+      if (finalDestLocSet.has(finalDestLoc)) {
+        logger.error(`there are multiple file build to the same ${finalDestLoc}`);
+        process.exit(1);
+      }
+      finalDestLocSet.add(finalDestLoc);
+

[Rich-Harris]

Yeah, that would definitely help in the snowpack build case. Would be super helpful if these collisions could be identified in dev as well

[FredKSchott]

@ZYSzys thanks for the PR! This was fixed in the latest v2.18.2 release

[Rich-Harris]

We just ran into this problem again — a single directory had an index.js alongside an index.svelte file. In build you get a helpful error, but in dev it fails in very cryptic ways.

It occurred to me that there's no reason these files need to clash. Instead of replacing all non-js extensions with js, why not just append the extension? So index.svelte becomes index.svelte.js, index.ts becomes index.ts.js and so on. This seems simpler, implementation-wise, but also preserves the sort of ambient information that can be very helpful when trying to debug stuff.

[FredKSchott]

Interesting! There is definitely something nice about keeping that info for debugging. Let me give a look to see how big of a lift that would be implementation-wise.

[FredKSchott]

This ended up being much easier than expected! PR created: #2014

Unfortunately, we ran into a limitation that breaks all TypeScript projects that I can't think of a good workaround for. From the PR:

BLOCKED: This exposes an issue for TypeScript: tsc forces you to either import a file using .js, .jsx, or no file extension at all. This means that the user sees an error when they write import './App.tsx'. Unfortunately I can't think of a good workaround here:

• import './App.tsx.js'; not viable, we want to support imports that are based on the file path and not the final URL
• import './App'; viable, but currently this results in a broken import './App.js' at build. We need to improve our handling of this to do a true lookup/resolve instead of a lazy + '.js', but even then you'd run into issues with conflicts between "App.svelte" and "App.js" if you can only import each using "./App", which was the whole issue we were trying to solve for with this.

We may want to escalate this feedback to the TS team, since I know this limitation is a known issue for them reported by both Deno and Google.

@Rich-Harris curious if you have any thoughts