-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
import
fails to resolve when using esbuild
build tool
#24593
Comments
import
fails to resolve when using esbuild build toolimport
fails to resolve when using esbuild
build tool
If you'd like esbuild to be more flexible in its ability to resolve package paths I recommend making an issue or PR to the esbuild project rather than one here. But either way it is not common for packages to export files in the style you have recommended. And as you've mentioned this kind of export can lead to file ambiguity. It's best to explicitly import the files you want to your project. You can see that even |
I thought omitting the .js extension on imports was mostly a TypeScript-ism, is it used outside TS? I don't think we should be doing that anywhere within this project, including examples... |
Yes... Again see here in react-dom. If people want to use a unique, non-standard style of file imports like this they'll need to configure their build system to support it. It can't be each packages responsibility to accommodate it. |
Explicit file extensions are required since threejs is an ES module. It's not correct to blame other tools for following node resolution, they're working as intended. |
I don't like extension-less imports because they are confusing (no one reading the code would be able to understand whether the code imports JS file, TS file, or even a CSS file) and they wouldn't work if there are multiple files with the same names but different extensions. |
Esbuild is following the ES Modules specification (not node), which requires the Other bundlers (like webpack) allow you to omit the |
Everyone seems to have their own reasons, so is the Does a For whether node allows to ignore file extensions, please refer to the official documentation here: --experimental-specifier-resolution=node I noticed that some people gave examples of Changing the |
@wang1212 the files you are importing have If you prefer to use that style of imports, and are not using a build tool that enables them by default, I think we have to ask you to configure your build tool accordingly. I do not think it's a good idea for packages like three.js to alias those rewrites themselves; we will cause more problems than we solve unfortunately. |
This isn't correct, and you shouldn't be consulting react internals as to how to configure packages; react is a very complex and involved codebase.
To clarify, NPM packages follow Node specification and resolution between ESM and CJS, and three ships examples as ESM-only. Some tools may transpile and allow omitting file extensions for interop (CJS doesn't have this restriction). Vite, based on rollup and esbuild, would be the most notable to do this due to its stack and use across frontend, backend, and testing. I'd recommend that for @wang1212 if their project allows it.
I see that the proposed alias was actually reverted via 528193f of r137. Why is that? Is it not worthwhile to support both methods of imports for JS via https://nodejs.org/api/packages.html#package-entry-points? This would also work with #24595. // Hypothetical but pertinent to #23368
"./addons": "./examples/jsm/index.js",
"./examples/fonts/*": "./examples/fonts/*",
"./addons/*": "./examples/jsm/*",
"./addons/*.js": "./examples/jsm/*.js",
"./examples/*": "./examples/jsm/*",
"./examples/*.js": "./examples/jsm/*.js", Otherwise, has this since been documented, or is there no need to? I'm not sure if it's a breaking change but a source of confusion for sure. |
My only point in that post was that there are large packages choosing to omit extensions in their imports and that it requires a specialized build system to support. |
I don't understand what you're saying. I'm sure there's been internal dialogue and patch releases around this (noting 528193f, specifically), but surely the above configuration wouldn't be such a maintenance concern. It is my understanding that it is purely beneficial for user-land. |
It was a response to #24593 (comment) asking whether this only happened in TS.
The extensions were removed within days of being the exports initially being added (and before a release was made) and likely because the previous implementation with .js precluded the ability to import files with the extension. Three.js has always been designed around writing, distributing, and encouraging code that can be run directly in the browser so I don't expect the project would have deliberately been enabling exports that encouraged extension-less paths that required a build process. There are lots of ways that people to structure and organize their code. If users have a preferred ergonomic way to import files then more power to them - but I don't think it's three.js' job to directly accommodate all of them. Users can configure their build process to support their ergonomic preferences. Otherwise in userland this is inconsequential. Everything still works all the same. If someone else on the project is interested in getting this changed added that's fine but I don't expect my opinions are all that divergent from the project opinions on the topic. |
How is it not strictly correct for a package published on NPM using an NPM-specific package.json file to prevent the most common and painful interop issue that presents a user-land gotcha through a seemingly trivial and supported configuration? I don't understand why this is reduced to ergonomics; it otherwise makes authoring on top of three, especially a library, very error-prone and confusing in sometimes unfixable ways, especially if you forgo a build tool that would abstract this away. This is why I bother to comment, and one of the concerns of pmndrs/three-stdlib, aside from tree-shaking, both of which I would really like to be able to contribute here and make redundant following #24595. I want three examples or addons to be easy and safe to use for end-users, particularly for modern stacks. What can I or we do to get there because I don't see an alternative? |
See #23347. Ultimately it was chosen to match the usage in a pure ES Modules environment (threejs.org/examples), while allowing bundlers to optionally allow no
This still doesn't work, the |
No, now we have an alias |
I'm not sure that I see how hiding the We already offer and support too many ways of installing dependencies, many of which are fragile. We've just added |
If the concern here is that users cannot import three.js examples modules using
Please make a new issue that clearly states the problem and provides practical examples of where it's failing. You have to understand that I (and seemingly the the rest of the three.js maintainers) do not use a comparable build process to what you're familiar with and in fact are working towards removing builds as much as possible for ourselves. So I do not know Vite, Next.js, etc. And I only know esbuild and webpack as much as I have to. I personally detest this ecosystem fragmentation and it's exhausting to deal with so I avoid it as much as possible. A lot of the discussions around this in the past have been plagued by this disconnect in communication so my only point here is that you need to make it easy for us to see and understand the failures and how it's impacting people in practical development because it is very not obvious at least to me, though I do understand that The importability of the examples has improved significantly in this regard over the last couple years so if there are still issues users I think it's worth raising again. I'm otherwise willing to make recommendations or suggest solutions that will hopefully be acceptable to the project. |
I'll volunteer as a different perspective on this — I strongly prefer ESM-based build tooling myself. I've personally stopped answering questions about installation unless both ESM and a bundler (or Node.js) are involved; the rest of the options feel exhausting for me. Future versions of three.js-related packages I maintain (e.g. three-pathfinding, three-filmic, three-to-cannon, mikktspace, and gltf-transform) will eventually stop publishing the CommonJS and UMD entrypoints, just publishing ESM instead. Exact timing TBD but probably not this year. That difference aside, I agree with @gkjohnson that aliasing |
Is this a desirable or sincere request (collectively)? Really. I have to ask because I'm getting mixed messages here, and I'm not hopeful for resolve in the interim that doesn't just point people to pmndrs/three-stdlib. Perhaps that's an acceptable workaround? I'd be happy to contribute integration tests or work towards fixing examples' tree-shaking where we can sidestep this entirely with an addons flat bundle (#23368), but I want to make it clear whether the project is accepting of these before investing the time. The latter, I think, is much more agreeable in any case. Maybe this is something for #24595. |
I value your optimism 😁 I hope that it looks like we can do that in a year or two but it's hard to believe when there is no broader community drive or effort (specifically on Node's part) to create a path to ESM-only development or support interoperability between commonjs and esm more fluidly... Perhaps projects removing support for commonjs over time the only way to get the all rolling.
I can only speak for myself but if this is still breaking in practical and common use cases for users then it should be addressed. I just can't tell you if it is or when it would be because it's not how I work. Which is why I'm asking for real world examples and explanations. I can support an effort to improve the projects understanding of the problem which hopefully leads to a compelling enough reason to include example cjs files in a future release - I just will not be the one to instigate it. I have had (begrudgingly) to adjust my other projects (three-mesh-bvh, etc) to support a cjs variants for similar requests, as well. I sympathize with the concern over wasted time, though, but hopefully an initial explainer issue won't require such an extensive amount of time. Perhaps @Mugen87 @mrdoob and @donmccurdy can express some interest here.
I think an uber-bundle, tests, and tree-shaking issues are orthogonal to the issue we're discussing. My understanding is that we're just talking about distributing commonjs-compatible "/examples" code |
@CodyJasonBennett I do not understand what problems and solutions your comments refer to in this thread. It doesn't seem like we are all talking about
I have no idea what "this" refers to in either sentence above. Please, another thread. 😅🙏 |
Sure, go for it! I don't think anyone here is opposing to any tree-shaking improvements. |
+1 for an issue to discuss, I think an explainer on the unsolved problems here would be very welcome. |
An explainer issue would be helpful here but my understanding is that the core of the issue is that using However some build processes enable this functionality but respect the explicit package.json export paths (which currently means an extension is required). And users get confused when they suddenly have to start typing a file extension for require snippets even though "require" typically does not require it which is where this aliasing discussion started. The former is the actual problem, though. This is where my understanding is at, at least. |
Hm, I hadn't read that out of the discussion above. The OP does not mention Node.js, other than the resolution algorithm that many web build tools also follow. To be honest I think that we'll be stuck with Obviously the whole situation is ... not great. But for reasons that do not seem clearly related to requiring the |
On Node.js — I'm finding that most of my ESM-related pain in Node.js these days is related to particular packages rather than the Node.js runtime itself, which seems quite capable of supporting ESM-only development. Examples would be projects like |
I would like to get rid of all the CJS code... by the end of the year... |
I think this goal is doable! Furthermore |
I don't understand why even this is so controversial with examples being ESM-only. I've yet to see a technical reason why, so I'm hesitant to do anything remotely related. Is #24593 (comment) supposed to clarify this? I don't know, I'm giving up on this one. If a user or author wants to ship example code to modern stacks without this headache, use three-stdlib, not three/examples.
That's ofc your prerogative, OGL does this, and it's a huge relief there. They also only use the exports field and do not employ a build step. This kills three-stdlib in regards to dual-package hazard since it cannot fork the core. This is particularly problematic for CJS packages that rely on shared globals like React, so this would still be problematic for Next.js specifically. Ideally, people can choose to transpile instead if they must ship CJS, but they miss out on flat bundles and tree-shaking. This is why I want to contribute those here since this is stuff we've already solved upstream (minus #23372) and removes the need for three-stdlib rather than a transpilation step altogether. A flat bundle addons (#23368), which relies on tree-shaking, is my compromise and why I mention it in this issue. It sidesteps all of the above. |
@CodyJasonBennett I really appreciate your work in three-stdlib and your experience with the JS ecosystem, but I have not been able to understand your comments written in this thread. We seem to be talking about different things. As far as I can tell the OP has nothing to do with CJS or ESM or Node.js — it's just a missing file extension in an import path. I feel like whatever discussion this thread is branching into would probably be most appropriate for ... ... or a new issue if needed. I would also be glad to see the CJS code removed, per the issue above. |
That is an error coming from enforcement of the ES Modules spec via Node resolution since examples are ESM only. Yes, this is just a missing file extension but it is not very trivial to fix this when a tool is involved. This can be solved with configuration in three or incidentally through a CJS flat bundle which three-stdlib provides (for other reasons). You've made it clear that three isn't interested in this configuration, and that there's a preference for strict ES Modules compliance (which is more future-proof with static web/CDN as three goes ESM-only), so this isn't further actionable. I apologize for convoluting the issue with three-stdlib and a proposal for tree-shaking and addons, I'll continue in #24595. |
What does this mean? Ambiguous statements like this are one reason why this hasn't been going anywhere. I want to help but there have been no concrete examples or way to understand what is breaking and when. Tell us what tool. Show when it's breaking and how. Give us a real example. Without anything concrete this just looks like a superficial change because, again, we are not using the same tools and workflow you might be. I understand it might be more complex that it initially looks but you have to help us help you. I'm not sure how else to put it other than I don't feel like there's being enough effort put into communicating the issue. And if we don't understand the problem then it will just get inadvertently broken in the future. |
Is your feature request related to a problem? Please describe.
When using the esbuild build tool, the import statement reports an error that cannot be resolved.
E.g:
This is because of the following configuration:
Although tools like rollup, webpack, etc. may be normal, obviously esbuild respects node specification standards more.
According to the node specification, in order for the esbuild tool to work properly, the file extension needs to be added:
Obviously when this happens, many people need to spend as much time as I do to analyze the cause of the problem.
In fact, the following configuration can also solve the problem:
In order to improve development efficiency (compilation speed),
esbuild
should have been widely used, can this problem be solved?see also evanw/esbuild#2518
Describe the solution you'd like
.js
), change theexports
configuration in package.json, for exampleDescribe alternatives you've considered
Additional context
The text was updated successfully, but these errors were encountered: