Skip to content
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

esbuild tries to resolve wrong css file name in any node_module folder with an upper case .tsx-component #3341

Closed
jonasem opened this issue Aug 25, 2023 · 7 comments

Comments

@jonasem
Copy link

jonasem commented Aug 25, 2023

After upgrading esbuild 0.17 -> 0.19 we experienced a weird behavior where esbuild will try to resolve .css-files for components in node_modules even though they are not imported, just merely by beeing on the disk.

We recreated this in a tiny project only installing react@18 and [email protected] and adding a folder in node_modules like this:

image

The error output is:

Could not read from file: /home/jonas/tmp/esbuild-bug/node_modules/foo/bar/A.css

Note that it looks for a big-letter A.css, while we have a small one, and the kicker is, it's not ever imported anywhere.

We tested this multiple times, just making a lower-Pascal-case css file matching an upper case .tsx file triggers this error.

@evanw
Copy link
Owner

evanw commented Aug 25, 2023

Please provide a way to reproduce the issue. You have not provided enough information here. Marking this issue as unactionable for now since I cannot do anything without a way to reproduce the issue.

@jonasem
Copy link
Author

jonasem commented Aug 25, 2023

Sure, you can try this repo I made to show the issue https://github.com/jonasem/esbuild-component-css-bug

Be sure to do npm install or npm ci and then run postinstalll

@hyrious
Copy link

hyrious commented Aug 25, 2023

You can enable logLevel: 'verbose' to see what actually happens in detail, then you will find this log:

⬥ [VERBOSE] Resolving import "./bar/AComponent" in directory "/node_modules/foo" of type "import-statement"
...
    Checking for file "AComponent"
    Checking for file "AComponent.jsx"
    Checking for file "AComponent.js"
    Checking for file "AComponent.css"
    Found file "AComponent.css"

So here are 2 things:

  • when resolving extension-less imports, esbuild searches for .css files, this may be wrong
  • esbuild's resolver is case-insensitive, this is ok. however it failed to work well on case-sensitive fs like on linux

see it live in repl

@jonasem
Copy link
Author

jonasem commented Aug 25, 2023

Thank you for the insight into how to debug this issue. So I didn't expect this behavior, but it make sense when I see the error. Renaming the file to uppercase gave an error during runtime which also makes sense with the given verbose log.I hope I don't have to import typescript files by giving the full extension though, esbuils didn't do that before in this case.

@hyrious
Copy link

hyrious commented Aug 25, 2023

The resolver correctly works back at version 0.17.19 repl. So I guess this is a bug introduced in 0.18.


I guess this was added in this commit, where esbuild should prefer .js files over .ts files in node_modules. However that commit also sorts .css before .ts, that's why the CSS file was chosen.

It should work by changing the orders, however the case-insensitive problem still exists.

@jonasem
Copy link
Author

jonasem commented Aug 28, 2023

Don't really know what more to make of this now. I wrote some tests in bundler_ts_test.go to confirm my thinking.

I only got a failing test on this, and no solution. But I suspect a .ts file in node_modules importing an implicit extension should favor .ts over .css.

Basically I would propose a change to resolve .css after .ts in node_modules, something along the lines of:

diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go
index 11647c6e..37185216 100644
--- a/internal/resolver/resolver.go
+++ b/internal/resolver/resolver.go
@@ -252,7 +252,7 @@ func NewResolver(call config.APICall, fs fs.FS, log logger.Log, caches *cache.Ca
        // for imports of files inside of "node_modules" directories
        nodeModulesExtensionOrder := make([]string, 0, len(options.ExtensionOrder))
        for _, ext := range options.ExtensionOrder {
-               if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() {
+               if loader, ok := options.ExtensionToLoader[ext]; !ok || !loader.IsTypeScript() || !loader.IsCSS() {
                        nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
                }
        }
@@ -261,6 +261,11 @@ func NewResolver(call config.APICall, fs fs.FS, log logger.Log, caches *cache.Ca
                        nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
                }
        }
+       for _, ext := range options.ExtensionOrder {
+               if loader, ok := options.ExtensionToLoader[ext]; ok && loader.IsCSS() {
+                       nodeModulesExtensionOrder = append(nodeModulesExtensionOrder, ext)
+               }
+       }
 
        // Generate the condition sets for interpreting the "exports" field
        esmConditionsDefault := map[string]bool{"default": true}

@evanw Is there anything I can do to help along with this issue?

@vprus
Copy link

vprus commented Jan 23, 2024

Hi,
I might be missing something, but what's preventing this issue from getting fixed - if there was a PR, even? I've just run into it in a project that has index.tsx + index.css in a number of directories, and esbuild picks CSS one, and fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants