fix: don't extract http:, https:, data: and hash only urls as assets

[yume-chan]

Fixes #233

Points to note:

1. data: URLs are actually extracted as asset imports (not ignored) before this change.
2. I also ignored http: and https: URLs, they should be external resources, not part of module resolution.
3. I can't find where prefixes are added, so -webkit-filter:url(packages/babel-preset/__fixtures__/assets/a.svg#a); is still incorrect.

[layershifter]

I can't find where prefixes are added, so -webkit-filter:url(packages/babel-preset/fixtures/assets/a.svg#a); is still incorrect.

Interesting that it's not processed in the same way, I will check later...

[github-actions]

📊 Bundle size report

_{🤖 This report was generated against e775fd8c168801f9f914c5a2810375ed498a4cb6}

[yume-chan]

I can't find where prefixes are added, so -webkit-filter:url(packages/babel-preset/__fixtures__/assets/a.svg#a); is still incorrect.

I know, the whole CSS block is passed into replaceAssetsWithImports as one string, ".fv04sme{-webkit-filter:url(packages/babel-preset/__fixtures__/assets/a.svg#a);filter:url(packages/babel-preset/__fixtures__/assets/a.svg#a);}" in this case, so it contains two url(), but the Regex only matched the later one.

It may be difficult to correctly parse the block.

[yume-chan]

Now it also has problem with multiple URLs like backgroundImage: "url(./a.png), url(./b.png)" (https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Backgrounds_and_Borders/Using_multiple_backgrounds), only the last one is extracted. maskImage and filter also support multiple values.

The best solution might be to use a CSS parser for semantically parsing the input and handle each rule differently. Considering the complexity, I don't think I will have time to complete that.

[layershifter]

The best solution might be to use a CSS parser for semantically parsing the input and handle each rule differently. Considering the complexity, I don't think I will have time to complete that.

Good call, I will look into it 👀

[layershifter]

The best solution might be to use a CSS parser for semantically parsing the input and handle each rule differently. Considering the complexity, I don't think I will have time to complete that.

No worries, I created an issue #238 and you can check a fix in #237. Once the fix will be merged I will rebase your changes 😉

[layershifter]

@yume-chan #237 was merged, do you need help with rebasing changes?

[yume-chan]

@layershifter I have rebased main into my branch and it's ready to be reviewed.

Some notes:

1. Hash only URLs are filtered by isAssetUrl now, makes code cleaner.
2. normalizeStyleRule doesn't check if the URL has hashes because require("node:path").resolve("/foo", "bar.svg#id") will correctly output "/foo/bar.svg#id" as we want
3. The URL string is replaced with normalized one even if it's not an asset, otherwise quoted values will not be de-duplicated and replaceAssetsWithImports will process them as assets (because they start with ")

[layershifter]

Really like how it was simplified, great job ❤️

[layershifter]

Released in @griffel/[email protected] & @griffel/[email protected] 🚢