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

Pure esbuild sass files with relative assets fails to load #48

Closed
Hideman85 opened this issue Dec 9, 2021 · 20 comments
Closed

Pure esbuild sass files with relative assets fails to load #48

Hideman85 opened this issue Dec 9, 2021 · 20 comments

Comments

@Hideman85
Copy link

I do have a relative import issue:

✘ [ERROR] Could not resolve "./NunitoSans-Bold.ttf"
    sass-plugin-0:/....../LoadingAlert.scss:51:11:

The file import a relative sass file

I found other issues on the repo like @import "../../../fonts/fonts.scss"; and this file is importing the font.

I do see other similar issues on the repo but I don't really get it... I try to find if this is due to dart-sass but no clue either....
Currently this is working with webpack but I do think like #19 this ise due to special resolver.

What can I do to solve my issue?

@glromeo
Copy link
Owner

glromeo commented Dec 10, 2021

I never saw an error message like that, can you help me reproduce this issue?
is that you obfuscating the path or is it really /....../ ???

If you could share the repo that would be great!

@Hideman85
Copy link
Author

Hideman85 commented Dec 10, 2021

Yes I just obfuscated the complete path to ..... I should have put *
The project is a closed source commercial project so I cannot share it but I could see to produce a reproduction case.

Edit: Reproduction https://github.com/Hideman85/esbuild-sass-test-case

  • yarn install
  • yarn node build.js

@glromeo
Copy link
Owner

glromeo commented Dec 12, 2021

This actually is a similar issue to #19 indeed...
This little debug plugin shows us that:

{
  name: "resolve-assets",
  setup(build) {
    build.onResolve({filter: /./}, ({path, resolveDir, importer, namespace})=>{
      console.log("resolve:", path, resolveDir, importer, namespace)
      return null;
    })
  }
}

the issue is not in the plugin but in esbuild trying and resolving the font url from the source file and not the sass file

resolve: D:/esbuild-sass-test-case/src/index.js D:/esbuild-sass-test-case  file
resolve: ./NunitoSans-Light.ttf D:\esbuild-sass-test-case\src D:\esbuild-sass-test-case\src\style.scss sass-plugin-0
X [ERROR] Could not resolve "./NunitoSans-Light.ttf"

    sass-plugin-0:D:\Workspace\esbuild-sass-plugin\test\issues\esbuild-sass-test-case\src\style.scss:3:11:
      3 │   src: url("./NunitoSans-Light.ttf");
        ╵            ~~~~~~~~~~~~~~~~~~~~~~~~

I can't take esbuild-sass-plugin in the direction taken by the webpack loader otherwise the build would end up
being equally slow and clunky.
The only way I can think of addressing these kind of import issues is by coding ad hoc plugins to rewrite the urls.
e.g.

    plugins: [
      sassPlugin({
        outputStyle: isProd ? 'compressed' : 'expanded'
      }),{
        name: "resolve-assets",
        setup(build) {
          build.onResolve({filter: /.ttf$/}, (args)=>{
            try {
              return {path: require.resolve(args.path, {paths: ['./fonts']})}
            } catch (e) {
              console.error(e)
              return null;
            }
          })
        }
      }
    ]

of course this solution is pretty poor but it might be the starting point for something that works for your project layout.

NOTE: The issue is that the importer of the asset (.ttf) is an information that sass has swallowed and neither the plugin or esbuild can have

@glromeo glromeo closed this as completed Dec 12, 2021
@Hideman85
Copy link
Author

Thanks for your complete answer, then I'm gonna open an issue in esbuild I don't like the resolve assets hand made fix it does not fit at scale.

@glromeo
Copy link
Owner

glromeo commented Dec 13, 2021

sorry @Hideman85 but esbuild author can't help you there!

NOTE: The issue is that the importer of the asset (.ttf) is an information that sass has swallowed and neither the plugin or esbuild can have

@Hideman85
Copy link
Author

But then this is not usable, we cannot tell all the users:

In order to use this plugin you have to manually resolve all your assets

@glromeo
Copy link
Owner

glromeo commented Dec 13, 2021

The issue is within SASS and a solution might come from sass/sass#2535
but for now you are probably better off using webpack and resolve-url-loader

@Hideman85
Copy link
Author

Yes currently I'm with webpack + babel + terser and that is horribly slow that's why I was looking to move to esbuild.
Do you think node-sass has the same issue? It might be an alternative too right?

@glromeo
Copy link
Owner

glromeo commented Dec 14, 2021

Node sass has the same issue but at least it would allow importers to intercept all @imports and one could hack into that to
rewrite relative urls in absolute that then esbuild could rewrite properly.
node-sass is deprecated so I cannot really invest all that effort to leverage on a feature that's seen by the sass team
as a bug: sass/dart-sass#574

This is the only hope: sass/sass#2535

by the way parcel and rollup have the same issues and what resolve-url-loader does is just crazy!

https://github.com/bholloway/resolve-url-loader/blob/v4-maintenance/packages/resolve-url-loader/docs/how-it-works.md

@Hideman85
Copy link
Author

Wow the resolve-url-loader is indeed crazy 😂

Our only hope rely on a issue opened more than 3 years ago 😢 I would say that it would probably happen a day but that does not look like for the near feature.

Maybe we can upvote the issue by adding our case and maybe it would boost it

@glromeo
Copy link
Owner

glromeo commented Dec 20, 2021

Hi @Hideman85 I just published https://www.npmjs.com/package/esbuild-sass-plugin/v/2.0.0-alpha.0 in npm.
In this version there are few breaking changes but I guess they won't affect your project.
Instead there's the replacement of the old renderSync API call with the new compile that allows importers to intercept all import resolutions, also the relative ones.
There's also a precompile option that allows re-writing the sass sources before they are compiled by sass which allows global variables but also, in your case, to rewrite urls.

Just switch to 2.0.0-alpha.0 and in you example configure the plugin like this:

plugins: [
      sassPlugin({
        outputStyle: isProd ? 'compressed' : 'expanded',
        // transform: postcssModules({
        //   // ...put here the options for postcss-modules: https://github.com/madyankin/postcss-modules
        // })
        precompile(source, pathname) {
          console.log("precompile:", pathname)
          return source.replace(/(url\(['"])(\.\.?\/)([^'"]+['"]\))/g, `$1${path.dirname(pathname)}/$2$3`)
        }
      })
    ]

I look forward to hear if it works in your codebase!

@glromeo glromeo reopened this Dec 20, 2021
@Hideman85
Copy link
Author

Hi @glromeo thanks for the update and your work 🙂
I just tried out and I cannot see the import error with the asset but it could be due to the flooded error that I have now.
Lot of my sass files import a relative _variables.sass to get access to the colors, margin and plenty of other variables for the styling.
But now I do have a import issue:

 [ERROR] ENOENT: no such file or directory, open 'tmp/project/src/_variables.scss'
  
1  @import "../../../variables";
           ^^^^^^^^^^^^^^^^^^^^
  
  - 1:9  root stylesheet

    src/components/SharedComponents/CardSelector.tsx:6:7:
      6  import './styles/CardSelectorStyles.scss'

I note that the logging do not contains a beginning / to the absolute path is that normal?

@glromeo
Copy link
Owner

glromeo commented Dec 20, 2021

hmm... the url() substitution shouldn't affect that... can you add the relevant files to https://github.com/Hideman85/esbuild-sass-test-case ?
This would help me debug the new importer logic and come up with a e2e test for a layout like yours

@Hideman85
Copy link
Author

Hideman85 commented Dec 20, 2021

I just ran my test case with dependencies up to date like:

  "devDependencies": {
    "esbuild": "~0.14.6",
    "esbuild-sass-plugin": "~2.0.0-alpha.0"
  },

And got this without the need of extra files:

└─$ yarn node build.js                                                                                                                                                         1 
precompile: /tmp/esbuild-sass-test-case/src/style.scss
 [ERROR] [plugin sass-plugin] ENOENT: no such file or directory, open 'tmp/esbuild-sass-test-case/fonts/fonts.scss'
  
1  @import '../fonts/fonts.scss';
           ^^^^^^^^^^^^^^^^^^^^^
  
  - 1:9  root stylesheet

    src/index.js:1:7:
      1  import './style.scss'
                ~~~~~~~~~~~~~~

(node:233087) UnhandledPromiseRejectionWarning: Error: Build failed with 1 error:
src/index.js:1:7: ERROR: [plugin: sass-plugin] ENOENT: no such file or directory, open 'tmp/esbuild-sass-test-case/fonts/fonts.scss'
  
1  @import '../fonts/fonts.scss';
           ^^^^^^^^^^^^^^^^^^^^^
  
  - 1:9  root stylesheet
    at failureErrorWithLog (/tmp/esbuild-sass-test-case/node_modules/esbuild/lib/main.js:1503:15)

I seen this:

yarn why esbuild
├─ esbuild-sass-plugin@npm:2.0.0-alpha.0
  └─ esbuild@npm:0.14.6 (via npm:^0.14.5)

└─ test-case@workspace:.
   └─ esbuild@npm:0.14.6 (via npm:~0.14.6)

Same output after downgrade, have you tired out? I'm on latest Ubuntu 21.10.

EDIT: The error is same without the precompile, does it mean the precompile is not doing the intended action?

EDIT2: Whatever I do it does not work 🤔 but putting back to "esbuild-sass-plugin": "~1.8.2" I get back the original issue

@glromeo
Copy link
Owner

glromeo commented Dec 20, 2021

looks an issue with the path separators...I am going to try it on my mac. Thanks for the help troubleshooting!
Your hunch was right! Win32 URLs have an extra / in front of an absolute path that but posix don't!

@glromeo
Copy link
Owner

glromeo commented Dec 20, 2021

@Hideman85
Copy link
Author

Hideman85 commented Dec 20, 2021

It does solve the issue but now it remains two points:

  • url(../asset.png) I guess I just need to update the regex to match urls without quotes
  • .myWrapper { @import '../someCssFile' } that is a common way to import built-in css content an rewrite it in a namespaced matter (you might need me to update my test case) The doc call it Nesting

EDIT: Test case updated

yarn node build.js
precompile: /tmp/esbuild-sass-test-case/src/style.scss
precompile: /tmp/esbuild-sass-test-case/fonts/fonts.scss
 [ERROR] [plugin sass-plugin] Stack Overflow
  
4    @import '../lib/SomeLib/builtIn'; // Note the extension should not be added, this is intended to use namespace feature
             ^^^^^^^^^^^^^^^^^^^^^^^^
  
  - 4:11  root stylesheet

    src/index.js:1:7:
      1  import './style.scss'
                ~~~~~~~~~~~~~~

@glromeo
Copy link
Owner

glromeo commented Dec 23, 2021

@Hideman85 I just released 2.0.0-alpha.3
which should have a pretty solid import resolution now... I look forward to your feedback!

@glromeo
Copy link
Owner

glromeo commented Dec 24, 2021

@Hideman85 v2.0.0 has been published... I close this issue now

@glromeo glromeo closed this as completed Dec 24, 2021
@Hideman85
Copy link
Author

Super thank you so much for your work, I will check out later :)

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

No branches or pull requests

2 participants