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

bytesInOutput from metafile is incorrect for css files since v0.12.12 #2071

Closed
somebee opened this issue Mar 2, 2022 · 6 comments
Closed
Labels

Comments

@somebee
Copy link

somebee commented Mar 2, 2022

UPDATE: bytesInOutput is actually incorrect whether you supply a publicPath or not. As long as you have a url(...) somewhere in your css that imports a path using file-loader.


When you supply a publicPath, the bytesInOutput value is incorrect for css files. Repository with minimal reproducible example can be found here: https://github.com/somebee/esbuild-bytes-offset-bug.

Take the input:

body {background-image: url(./image.svg);}

When specifying a publicPath /some-long-public-path/, esbuild generates the following:

/* styles.css */
body {
  background-image: url(/some-long-public-path/image-WFRGLPG5.svg);
}

And metafile:

"outputs": {
   ...
    "dist/index.css": {
      "imports": [],
      "inputs": {
        "styles.css": {
          "bytesInOutput": 61
        }
      },
      "bytes": 94
    }
  }

bytesInOutput of styles.css is shorter than the real output, as it does not include the /some-long-public-path in the calculation. The real bytesInOutput is 76

@somebee
Copy link
Author

somebee commented Mar 2, 2022

After some more investigation it seems like the bytesInOutput is incorrect even when not supplying a publicPath. It is incorrect whenever the css files include a url reference. I guess the bytesInOutput is calculated before transforming the paths or something.

The bytesInOutput are not at all affected by --asset-names, --public-path or any other option that ends up changing the urls in the output css. So, whenever you have assets in css-files you can not trust the bytesInOutput to be able to extract specific parts of the output.

If you change the .svg loader to be dataurl the bytesInOutput works as expected.

@evanw
Copy link
Owner

evanw commented Mar 2, 2022

But publicPath comes from the esbuild configuration, not from the file. So you could make the argument that the bytesInOutput should not attribute those bytes to the file.

@somebee
Copy link
Author

somebee commented Mar 2, 2022

Well, minify and a bunch of other options also comes from the esbuild configuration but they predictably affect the bytesInOutput. With an example of a few assets, and no configuration except minify I get this output in the metafile:

"dist/index.css": {
      "imports": [],
      "inputs": {
        "style.css": {
          "bytesInOutput": 201
        }
      },
      "bytes": 177

It does seem like a bug. Did not happen on v0.9.7. I have not yet worked my way backwards to see when it started reporting incorrect bytesInOutput. As I mentioned in the followup comment, this "bug" happens without any configuration, no publicPath or anything else. All you need is to refer to an asset that is configured to use the file-loader.

I need the correct bytes to be able to after the bundling re traverse the tree and create a merged css file containing chunks in correct order from multiple entrypoints (server and clients), in the imba bundler. Our custom logic worked perfectly for this when we used v0.9.7. Just trying to update due to all the other fantastic improvements that have come along since then.

If this is sort of working as intended, I can see us bypassing the bytesInOutput by instead intercepting onLoad for all css files and add a legal comment at the top and bottom, together with --legal-comments=inline and then extract the chunks based on those markers.

Btw esbuild is the most impressive open-source project I've had the pleasure to use in many years. I cannot thank you enough for how fantastic it has been for us and how well it works in the imba bundler.

@somebee
Copy link
Author

somebee commented Mar 3, 2022

About to start undertaking the hack where we inject legal comments in all css inputs to get around this, so would love to hear if you think this will be resolved at some point or if I should rather explore workarounds :)

If it was possible to explicitly turn off minification just for the css I could also use the header comments above each input to extract the chunks, but afaik there is no option to do this.

@evanw: would it be helpful if I figured out the exact version where it started breaking?

@somebee
Copy link
Author

somebee commented Mar 3, 2022

Improved the testcase in the mentioned repo and found out that the bug was introduced in v0.12.12. Again, this happens without any configuration at all.

@somebee somebee changed the title bytesInOutput from metafile does not take publicPath into account bytesInOutput from metafile is incorrect for css files since v0.12.12 Mar 3, 2022
@haikyuu
Copy link

haikyuu commented Mar 9, 2022

I have created a PR that solves the issue.

Issue statement

bytesInOutput has an incorrect value when an url(./images/image.png) is used is a CSS file. That's because it's calculated based on the result of the printer and that uses a hash inside the url instead of the actual path.

@evanw I'd love to have a review on the PR if possible. It's quite small and has a couple of tests.

@evanw evanw added the metafile label Dec 4, 2022
@evanw evanw closed this as completed in a375b37 Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants