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

Enable nohoist by default #223

Merged
merged 1 commit into from
Nov 1, 2023
Merged

Enable nohoist by default #223

merged 1 commit into from
Nov 1, 2023

Conversation

malash
Copy link
Collaborator

@malash malash commented Nov 1, 2023

In #120 it said nohoist.enable is set to false in development mode to speed up code bundling.
But recently we meet an unexpected issue, that some global CSS styles in a page polluted other page. Consider we have a sub-package packageA and a page packageA/pages/index inside it with these CSS:

:global() {
  page {
    background-color: red;
  }
}

If nohoist.enable was false, these lines of code would be bundled into _goji_commons.wxss and shared by all pages. Which means all pages' background became red.

Therefore, I decided to enable this option by default in case of different behavior on production and development mode.

Also, I tested and found the bundling performance affect is acceptable. See the result of re-bundling time costs ( in ms ) in a large GojiJS project.

nohoist.enable false true Delta
Re-bundling test 1 2312 2358
Re-bundling test 2 1952 1840
Re-bundling test 3 2014 2032
Re-bundling test 4 1779 2116
Re-bundling test 5 1928 1886
Re-bundling test 6 1882 1927
Re-bundling test 7 2012 1794
Re-bundling test 8 1902 1848
Re-bundling test 9 1894 1907
Re-bundling test 10 1939 2029
Average 1961.4 1973.7 0.63%
Truncated mean 1940.375 1948.125 0.40%

@malash malash merged commit 78d309b into main Nov 1, 2023
18 checks passed
@malash malash deleted the enable-nohoist-by-default branch November 1, 2023 07:35
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