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

Swallow fingerprint from css asset urls #50

Merged
merged 1 commit into from
Feb 19, 2022
Merged

Swallow fingerprint from css asset urls #50

merged 1 commit into from
Feb 19, 2022

Conversation

brenogazzola
Copy link
Collaborator

@brenogazzola brenogazzola commented Dec 12, 2021

Partly closes #47.

This PR makes the CSS compiler regex swallow the fingerprint after ? in asset urls.

Benchmark indicates a 3% slowdown.

@HLFH
Copy link

HLFH commented Jan 21, 2022

Not enough to fully fix the issue.

Currently using this issue-47 branch from #50.
This is what I get in my error logs:

Unable to resolve 'fa-brands-400.svg#fontawesome' for missing asset 'fa-brands-400.svg#fontawesome' in page.css
Unable to resolve 'fa-regular-400.svg#fontawesome' for missing asset 'fa-regular-400.svg#fontawesome' in page.css
Unable to resolve 'fa-solid-900.svg#fontawesome' for missing asset 'fa-solid-900.svg#fontawesome' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css
Unable to resolve 'fa-brands-400.svg#fontawesome' for missing asset 'fa-brands-400.svg#fontawesome' in page.css
Unable to resolve 'fa-regular-400.svg#fontawesome' for missing asset 'fa-regular-400.svg#fontawesome' in page.css
Unable to resolve 'fa-solid-900.svg#fontawesome' for missing asset 'fa-solid-900.svg#fontawesome' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css

By using the main branch:

Unable to resolve 'fa-brands-400.eot?#iefix' for missing asset 'fa-brands-400.eot?#iefix' in page.css
Unable to resolve 'fa-brands-400.svg#fontawesome' for missing asset 'fa-brands-400.svg#fontawesome' in page.css
Unable to resolve 'fa-regular-400.eot?#iefix' for missing asset 'fa-regular-400.eot?#iefix' in page.css
Unable to resolve 'fa-regular-400.svg#fontawesome' for missing asset 'fa-regular-400.svg#fontawesome' in page.css
Unable to resolve 'fa-solid-900.eot?#iefix' for missing asset 'fa-solid-900.eot?#iefix' in page.css
Unable to resolve 'fa-solid-900.svg#fontawesome' for missing asset 'fa-solid-900.svg#fontawesome' in page.css
Unable to resolve 'et-line.eot?#iefix' for missing asset 'et-line.eot?#iefix' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css
Unable to resolve 'et-line.eot?#iefix' for missing asset 'et-line.eot?#iefix' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css
Unable to resolve 'themify.eot?-fvbane' for missing asset 'themify.eot?-fvbane' in page.css
Unable to resolve 'themify.eot?#iefix-fvbane' for missing asset 'themify.eot?#iefix-fvbane' in page.css
Unable to resolve 'themify.woff?-fvbane' for missing asset 'themify.woff?-fvbane' in page.css
Unable to resolve 'themify.ttf?-fvbane' for missing asset 'themify.ttf?-fvbane' in page.css
Unable to resolve 'themify.svg?-fvbane#themify' for missing asset 'themify.svg?-fvbane#themify' in page.css
Unable to resolve 'fa-brands-400.eot?#iefix' for missing asset 'fa-brands-400.eot?#iefix' in page.css
Unable to resolve 'fa-brands-400.svg#fontawesome' for missing asset 'fa-brands-400.svg#fontawesome' in page.css
Unable to resolve 'fa-regular-400.eot?#iefix' for missing asset 'fa-regular-400.eot?#iefix' in page.css
Unable to resolve 'fa-regular-400.svg#fontawesome' for missing asset 'fa-regular-400.svg#fontawesome' in page.css
Unable to resolve 'fa-solid-900.eot?#iefix' for missing asset 'fa-solid-900.eot?#iefix' in page.css
Unable to resolve 'fa-solid-900.svg#fontawesome' for missing asset 'fa-solid-900.svg#fontawesome' in page.css
Unable to resolve 'et-line.eot?#iefix' for missing asset 'et-line.eot?#iefix' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css
Unable to resolve 'et-line.eot?#iefix' for missing asset 'et-line.eot?#iefix' in page.css
Unable to resolve 'et-line.svg#et-line' for missing asset 'et-line.svg#et-line' in page.css
Unable to resolve 'themify.eot?-fvbane' for missing asset 'themify.eot?-fvbane' in page.css
Unable to resolve 'themify.eot?#iefix-fvbane' for missing asset 'themify.eot?#iefix-fvbane' in page.css
Unable to resolve 'themify.woff?-fvbane' for missing asset 'themify.woff?-fvbane' in page.css
Unable to resolve 'themify.ttf?-fvbane' for missing asset 'themify.ttf?-fvbane' in page.css
Unable to resolve 'themify.svg?-fvbane#themify' for missing asset 'themify.svg?-fvbane#themify' in page.css

The ones with the number sign without the question mark are not taken into account.

@brenogazzola
Copy link
Collaborator Author

Updated to add the # test. @HLFH could you check again please?

@kevynlebouille
Copy link
Contributor

@brenogazzola Things running well for me now!

Sample with propshaft (lastest release with ASSET_URL_PATTERN override): http://localhost:3000/assets/fonts/bootstrap-icons-1ca5e8c7d2fb8e9d60ad1a1feb2a46e98c248a3d.woff2 => 200 OK

Sample with sprockets: http://localhost:3000/assets/fonts/bootstrap-icons-4aee98800746975593dace855d8dc1b9101f7b66df718a65812d1caae06579a0.woff2?524846017b983fc8ded9325d94ed40f3 => 200 OK

Thx

@dhh dhh merged commit 1ba2986 into main Feb 19, 2022
@dhh dhh deleted the issue-47 branch February 19, 2022 09:52
@brenogazzola brenogazzola mentioned this pull request Mar 2, 2022
@HLFH
Copy link

HLFH commented Mar 15, 2022

@brenogazzola Works well. Thank you.

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.

Importing libraries with fonts from node_modules (Bootstrap Icons example)
4 participants