-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix(dev): avoid FOUC when swapping out link tag (fix #7973) #8495
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation looks good to me.
Would you rebase this to the main branch? |
@sapphi-red thank you so much for the review. I am happy to rebase against |
Thanks for the PR, this is a big DX improvement! I think we should merge it and try it first against main, and we can then backport it to v2 |
Sounds good. I'll get onto this tomorrow. Finishing up on my end. Thanks for checking it out! |
Assuming we have a CSS file at the following path... ``` /resources/css/app.css ``` When adding a link tag to the DOM manually (i.e. via Server Side Rendering)... ```html <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css"> ``` Vite will now watch this file and hot reload changes in the browser for this asset. When we update `app.css` Vite will update the link tag's `href` attribute like so... ```diff - <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?"> + <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?t={TIMESTAMP}"> ``` This is great, but for a split moment there is a flash of unstyled content as the asset has not loaded yet. The browser unloads the styles from the original stylesheet and waits until the new stylesheet has loaded and been parsed before it will show the new stylings. This PR addresses this problem. Now the process is as follows.. First the client adds a second (cloned) link tag to the DOM, leaving the original in place and still loaded by the browser... ```diff <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?"> + <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?t={TIMESTAMP}"> ``` Once the new stylesheet has loaded, the browser will remove the original stylesheet from the DOM, leaving the new _loaded_ stylesheet in the DOM. ```diff - <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?"> <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?t={TIMESTAMP}"> ``` This process then continues for future updates to the CSS file. I have no idea where to start adding a unit test for this. I'm a dumb back-end developer. Send help! Edit: i've added a test. Not sure if it is any good though!
@sapphi-red @patak-dev Rebased against main as requested 👍 |
Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM 🚀
Co-authored-by: 翠 / green <green@sapphi.red>
We backported this PR to the v2 branch. Is #6649 needed in 2.9 for your needs? An issue with that PR is that it may break integrations that aren't expecting
|
@patak-dev Thanks! This PR was the main blocker for us so we've very grateful for it being backported to 2.9. Do you have an ETA on 2.9.11? We're ready to launch once that's in. #6649 would be nice in 2.9, but we can work around it by augmenting the manifest from our plugin. |
Really appreciate your feedback and review on this folks ❤️ and thank you for putting your energy into Vite - we love it. |
@jessarcher released vite@2.9.11, good luck with the launch 🚀 Let's keep working on #6649 and merge that one in v3. The next steps would be to get you in vite-ecosystem-ci, so we can check that future releases of Vite won't break you before it happens. You only need to PR a test file here, like this one we already have for the community maintained plugin: https://github.com/vitejs/vite-ecosystem-ci/blob/main/tests/laravel.ts, and we will start getting reports on Vite Land #ecosystem-ci channel. Right now all the efforts of vite-ecosystem-ci are placed on releasing v3, if you have breaking changes you can create a @timacdonald back to you! We really appreciate you working closely with us and upstreaming as much as possible so everyone benefits. Vite has grown a lot over the last year because everyone in the ecosystem using it got involved and push it forward. I'm very happy to see Laravel officially embracing Vite as an option, I think it will help both ecosystems moving forward. |
Description
Assuming we have a CSS file at the following path...
When adding a link tag to the DOM manually (i.e. via Server Side Rendering)...
Vite will now watch this file and hot reload changes in the browser for this asset (very sweet!). When we update
app.css
Vite will update the link tag'shref
attribute like so...This is great, but for a split moment there is a flash of unstyled content as the asset has not loaded yet. The browser unloads the styles from the original stylesheet and waits until the new stylesheet has loaded and been parsed before it will show the new stylings.
This PR addresses this problem. Now the process is as follows..
First the client adds a second (cloned) link tag to the DOM, leaving the original in place and still loaded by the browser...
<link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?"> + <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?t={TIMESTAMP}">
Once the new stylesheet has loaded, the browser will remove the original stylesheet from the DOM, leaving the new loaded stylesheet in the DOM.
- <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?"> <link rel="stylesheet" href="{DEV_SERVER_URL}/resources/css/app.css?t={TIMESTAMP}">
This process then continues for future updates to the CSS file.
Additional context
I have no idea where to start with adding a unit test for this. I'm a dumb back-end developer. Send help! (I've read the guides - I just don't know how to actually write the test itself for something like this, sorry!).I added a test. I have no idea if it is good or not!
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).