Skip to content

[devtool] inject css as styleTag rather than singletonStyleTag #81297

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 6, 2025
+4 −1
Merged

[devtool] inject css as styleTag rather than singletonStyleTag #81297

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3fb89f
[devtool] inject styleTag instead of singletonStyleTag
huozhi Jul 4, 2025
dc3ff14
update comment
huozhi Jul 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/next/next-devtools.webpack-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ module.exports = ({ dev, ...rest }) => {
{
loader: 'style-loader',
options: {
injectType: 'singletonStyleTag',
Copy link
Contributor

@RobPruzan RobPruzan Jul 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats the difference, why does it fix the race?

will insert multi styleTag which can force it layout again when the style is added

so maybe a clarification for this point ^

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment to explain and explicitly set it to styleTag. It's also the default behavior tho, we don't need to worry much

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this should stop our setTimeout case from being needed? So, I see it as either:

  • the styleTag makes it so we never hit the edge case, so we should be able to remove the setTimeout case
  • if we aren't sure styleTag fixes all cases, we keep the setTimeout incase its in needed (but we know it will be buggy- better buggy than completely broken)

so it seems like we should either

  • document that the setTimeout case is just a safety fallback
  • remove the setTimeout case since its not needed
  • use some explicit reactive system vs settimeout (some callback synchronously called when the style is applied, and that callback applies styles)

this conclusion depends on if my initial understanding is correct. Maybe im missing something important?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setTimeout and the inect type are 2 different cases:

  • We still need the setTimeout as when there's no shadow root under the portal, you can't insert anything. The setTimeout was for waiting that. If you directly insert nothing will happen.
  • The injectType is to split one style tag into multi style tags. If you insert one and update it later, the styles updates might not be apply to the DOM. The styles are inserted but not picked up by browser.

// Explicitly set the injectType to 'styleTag' which is also the default behavior.
// We've experienced `singletonStyleTag` that the later updated styles not being applied.
// Keep using `styleTag` to ensure when new styles injected the style can also be updated.
injectType: 'styleTag',
insert: require.resolve(
'./src/build/webpack/loaders/devtool/devtool-style-inject.js'
),
Expand Down
Loading