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

Minify and hash inject_styles.js #1524

Merged
merged 1 commit into from Sep 18, 2020
Merged

Conversation

@benmccann
Copy link
Contributor

@benmccann benmccann commented Sep 17, 2020

inject_styles can be emitted as a chunk instead of an asset since it's a .js file. This allows it to be minified, which makes a better use of bandwidth. It also makes it have a hash in it's name, which is important for avoiding caching bugs. URLs under the /client path are cached. I potentially wanted to change the contents of inject_styles while coming up with a fix for #1505 (e.g. as in #1525), but that wouldn't work if the file is cached because users would get the old version of the file

Also, I think it would be nice to have inject_styles be processed by Babel. This seems like it would be necessary for that to happen. However, from my testing, it does not appear sufficient

@benmccann benmccann force-pushed the benmccann:minify-and-hash branch from 3fb05ed to cf35648 Sep 17, 2020
@benmccann benmccann added the bundling label Sep 17, 2020
@benmccann benmccann force-pushed the benmccann:minify-and-hash branch from cf35648 to 704e358 Sep 17, 2020
@benmccann benmccann mentioned this pull request Sep 17, 2020
@ArekBartnik
Copy link
Contributor

@ArekBartnik ArekBartnik commented Sep 18, 2020

@benmccann page speed also points that inject_styles.js should be preloaded.

@benmccann
Copy link
Contributor Author

@benmccann benmccann commented Sep 18, 2020

Ah, good call out. The work on this is never done :-p

I'm going to go head and merge this one to unblock #1525

@benmccann benmccann merged commit b162c63 into sveltejs:master Sep 18, 2020
10 checks passed
10 checks passed
Tests (10, ubuntu-latest)
Details
Tests (10, windows-latest)
Details
Tests (10, macOS-latest)
Details
Tests (12, ubuntu-latest)
Details
Tests (12, windows-latest)
Details
Tests (12, macOS-latest)
Details
Tests (14, ubuntu-latest)
Details
Tests (14, windows-latest)
Details
Tests (14, macOS-latest)
Details
Lint
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.