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

Remove --public-path from build_script #204

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

coezbek
Copy link

@coezbek coezbek commented Oct 8, 2024

This PR removes the '--public-path=/assets` option from the generator (install.rb).

Background: The esbuild option --public-path is meant to be used to define alternate absolute asset hosts for file loaders. The documentation under https://esbuild.github.io/api/#public-path gives an example of --public-path=https://www.example.com/v1.

Using it to prefix /assets is incorrect usage I believe and unnecessary.

When esbuild encounters a url(...) it will add the prefix to the rewritten url, but it will not impact the file location where the asset is stored by esbuild.

Setting --public-path=/assets will thus interfere with downstream asset processing (for example Sprocket's AssetUrlProcessor), because the corresponding asset can't be found on disk. The asset processor is looking to resolve the prefixed URL, but can't find it because esbuild isn't adding the public path to files written to outdir.

I believe this went unnoticed, because using this option only affects --loader:xxx=file (for instance when handling ttf or woff2) and when using esbuild to handle such assets at all (for instance when importing CSS via yarn).

Since asset urls are rewritten anyway, removing --public-path doesn't have a negative impact.

The esbuild option `--public-path` is used to define absolute asset hosts for file loaders. Setting `--public-path=/assets` will interfere with downstream asset processing (for example Sprocket's AssetUrlProcessor), because the corresponding asset can't be found on disk.

Since asset urls are rewritten anyway, removing --public-path doesn't have a negative impact.

https://esbuild.github.io/api/#public-path
@coezbek coezbek closed this Oct 8, 2024
@coezbek
Copy link
Author

coezbek commented Oct 8, 2024

I researched more about this and I am certain that that using --public-path is just wrong in this situation.

BUT: It seems there is too much hackery already going on relying on this, in particular javascripts imports are using this to link to files. So it is not likely that anybody will accept this change.

I will do some more research on how to solve the situation.

One way that helps to improve the situation is to tell esbuild to append .digested to filenames, so Sprocket doesn't apply its own fingerprinting (which breaks because sprocket can't find the files due to the wrongly append /assets/ path):

    "build": "esbuild app/javascript/*.* --asset-names=[name]-[hash].digested ..."

@coezbek
Copy link
Author

coezbek commented Oct 9, 2024

I looked into this some more and realized that sourcemaps are broken out of the box in 7.2.1 with Sprockets due to the public path setting.

Just run:

rails new app -j esbuild; cd app; bin/rails assets:precompile ; tail public/assets/application-*.js

The sourceMappingURL was removed and replaced by a semicolon.

Fundamentally, adding --asset-path option changed what the js bundling step is doing. Rather than allowing the downstream asset pipeline to rewrite the paths and digest them, the paths are now already rewritten and can't be resolved locally anymore without using heuristics.

@coezbek coezbek reopened this Oct 9, 2024
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.

1 participant