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

accurate bytesInOutput for css that includes urls #2091

Closed
wants to merge 3 commits into from

Conversation

haikyuu
Copy link

@haikyuu haikyuu commented Mar 8, 2022

This fixes #2071

Problem statement

Bytes in output is based on len(compileResult.CSS) which is the result of the printer. And it includes the hash inside the url instead of the actual path.

Example: div{ background: url(hashthats25characterslong); }

That leads to a wrong value for bytesInOutput

Solution

Right before adding bytesInOutput to the meta file, we loop over files that have parts in the current chunk (i.e that are imported from the current chunk):

  • calculate the final import path
  • add the offset (id - path) to the total offset
  • substract the offset from the length of the compiled result.

This is my first PR in esbuild, so I don't have enough knowledge about the internals to make a cleaner fix. But here are some thoughts/questions I had while working on this:

  1. The length of the hash is 25 bytes, so I hardcoded that. I didn't find a reusable utility to build a hash in the codebase. But that makes sense since it's only used in the bundler and the linker like so fmt.Sprintf("%sA%08d", args.uniqueKeyPrefix, args.sourceIndex) with A for asset and 8 bytes int representing the index of the asset + the unique key prefix that's 16 bytes long. --> that's 25 bytes
  2. Writing the metafile is done lazily in a couple of places using the chunk.jsonMetadataChunkCallback with a parameter that's the final output size. So it is possible to add a similar function for bytes in output. But it's useless unless we can avoid calculating the offset and defer calculating the bytes in output to a later stage

@somebee
Copy link

somebee commented Mar 17, 2022

@evanw Any way you could look at merging this?

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

Successfully merging this pull request may close these issues.

bytesInOutput from metafile is incorrect for css files since v0.12.12
2 participants