kazijawad / esbuild-plugin-svgr

An esbuild plugin for importing SVGs as React components.
https://npmjs.com/package/esbuild-plugin-svgr
MIT License
36 stars 12 forks source link

Prevents the usage of `.svg` files from stylesheets via `url()` #10

Closed hibachrach closed 1 year ago

hibachrach commented 1 year ago

Running into this problem when using the plugin

✘ [ERROR] Cannot use "some/path/to/my.svg" as a URL                                                                                                                                                                                

    path/to/stylesheet.css:14601:30:                                                                                                                                                                                             
      14601 │   background-image: none, url("some/path/to/my.svg");
            ╵                               ~~~~~~~~~~~~~~~~~~~~~

The underlying cause is because this plugin treats all ResolveKinds as equivalent, meaning that url()s in CSS files will be referencing JS files which errors out.

The fix would be to likely restrict to handling just the following

import-statement
require-call
dynamic-import
require-resolve

as these will actually be from within JS by writing an onResolve fn that narrows the transformation.

Would a PR be accepted?

kazijawad commented 1 year ago

Thank you for flagging this! Yes, if you're able to submit a PR, I can create a new release for it or I can try to fix the issue.

smol-honk commented 1 year ago

@kazijawad @hibachrach Created a PR that hopefully fixes this problem! let me know if there's anything you'd like me to update.

kazijawad commented 1 year ago

Thank you for your changes @smol-honk.

After looking into it a bit more... I wonder if the default case should only return external: true as done here: https://github.com/kazijawad/esbuild-plugin-svgr/blob/main/index.js#L16-L18

I don't think we want to assume the user will always want to resolve it as an absolute path, but let me know if I'm missing something.

smol-honk commented 1 year ago

@kazijawad that sounds good to me! I think i added specific paths because I wanted to make sure it grabbed the right asset, but I think it's totally dependent on how the project is structured and not necessarily a svgr plugin situation.

kazijawad commented 1 year ago

I published a new version of the plugin, which should resolve this issue. I'm going to go ahead and close it now, thank you both for the help!