-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ChunkNames collision when dynamically importing files with same name in different directory #1099
Comments
I think the problem here is that the
It's supposed to do this already: esbuild/internal/bundler/linker.go Lines 4965 to 4966 in 6f6b060
That sounds perfect. Thanks! |
Okay actually I found a simple-ish way to repro. Here's a repository: https://github.com/Jarred-Sumner/dynamic-import-repro/tree/main Just run Debug log
If the manifest is a file that exists on the filesystem, the bug doesn't happen. If the manifest is returned by a plugin, it does happen. Whether or not the imports are absolute or relative paths are irrelevant |
Ah that makes sense It seems specific to the onResolve callback. If I instead have onResolve return a path to a file, the behavior is the same as the onLoad callback.
Tangent: from looking at the debug level logs -- if an entry point starts with |
Well people don't expect it to work this way. But it's a deliberate aspect of esbuild's directory cache. You can read more about it here: #254 (comment). Edit: What you're probably seeing is searches for files named |
Oh, I didn't realize how many filesystem lookups bundlers need to do. The node module resolution algorithm is a good developer experience but seems to slowly mimic text editor autocomplete.
After testing more, this isn't true. The plugin seems to have absolutely nothing to do with the issue. If I rename the file from The strange thing is that the contents of each of these three files being dynamically imported are different. More notes:
import * as hi from "./hi/[name]";
import * as name from "./[name]";
import * as index from "./index";
export default {
hi,
name,
index,
}; Also, thanks for looking at the (many) issues I've posted at this point. |
Workaround: If you create a file with a different filename that just So instead of this: export const hi = () => import("./hi/[name]");
export const name = () => import("./[name]");
export const index = () => import("./index");
export default {
hi,
name,
index,
}; You can do this: export const hi = () => import("./hi/[name]");
export const name = () => import("./[name]-stub");
export const index = () => import("./index");
export default {
hi,
name,
index,
}; And export * from "./[name]"; |
Thanks for all of the detail. Turns out this was caused by an incorrect optimization that tried to skip hashing when it wasn't needed (or at least an optimization that is now incorrect since dynamic entry points now use |
Thanks for fixing!
Doubt this would make up for the performance difference but I wonder if using xxHash instead of SHA128 would be faster. There's a Go implementation |
Thanks for the pointer. It looks like the pure Go implementation of xxHash reduces the hash time for the benchmark from 25ms to 4ms (6x faster) which is pretty great. I did some reading and I think the trade-offs for it sound good too (better at large sizes, designed to be fairly collision-resistant). I think it's worth a try. |
BTW Webpack is migrating to xxhash64 by default too |
Wyhash seems slightly better than xxhash but not as big of an improvement
relatively
https://github.com/wangyi-fudan/wyhash
Zig uses it for standard library hashtable
…On Thu, Oct 14, 2021 at 5:38 AM LifeIsStrange ***@***.***> wrote:
BTW Webpack is migrating to xxhash3 by default webpack/webpack#14306
<webpack/webpack#14306>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1099 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFNGSZUJ2VLJ4U4BL5TXE3UG3FLNANCNFSM42H27SZA>
.
|
right, wyhash probably is the second fastest hash function. The #1 being https://github.com/tkaitchuck/aHash , rust use it for its standard library hashbrown |
Exception
> error: Two output files share the same path but have different contents: static/chunk/[slug]-NCFBE2JN.js
This is with
BuildConfig.ChunkNames
set like this (in Go):Context
Say I have a dozen-ish pages like this:
To facilitate client-side routing with code splitting, it code-generates a manifest like this:
This one manifest is imported into each entry point and loaded via a plugin. Pages are lazily added to
BuildConfig.EntryPointsAdvanced
on request (in development). When pages are eagerly added toBuildConfig.EntryPointsAdvanced
, this exception does not occur. Note that since its a single manifest for the configuration, there is a cyclic dynamic import but it is never called at runtime.For a reproducible example, would an invite to a repo be okay?
Suggestion
Maybe a
[dir]
option forChunkNames
is warranted for cases like this? Or maybe the[hash]
could include the relative path of the files the chunk originated from?The text was updated successfully, but these errors were encountered: