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

PostCSS css-compile-variables is flakey with vite@2.9.0+ #722

Closed
MadLittleMods opened this issue Apr 19, 2022 · 9 comments · Fixed by #693
Closed

PostCSS css-compile-variables is flakey with vite@2.9.0+ #722

MadLittleMods opened this issue Apr 19, 2022 · 9 comments · Fixed by #693
Assignees
Labels
bug Something isn't working sdk

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Apr 19, 2022

With vite@2.9.1 installed, I'm noticing that sometimes yarn build:sdk fails with the following error related to scripts/postcss/css-compile-variables.js. I'm able to reproduce on the latest master with the changes from #721 to get around the first error noted in that linked PR.

I'm only able to reproduce on Windows 10. I tried running while yarn build:sdk; do :; done on macOS to repeat until failure but never saw any failure there.

Failing run ❌:

$ C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\.bin\vite build -c vite.sdk-assets-config.js
[vite:css] Cannot derive from background-color-secondary because it is neither defined in config nor is it an alias!
file: C:/Users/MLM/Documents/GitHub/element/hydrogen-web/src/platform/web/ui/css/themes/element/theme.css?type=runtime
error during build:
Error: Cannot derive from background-color-secondary because it is neither defined in config nor is it an alias!
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\src\platform\web\ui\css\themes\element\timeline.css:24:5
    at resolveDerivedVariable (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\vite.sdk-assets-config.js:280:19)
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\vite.sdk-assets-config.js:328:36
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:72:18
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:55:18
    at Rule.each (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:41:16)
    at Rule.walk (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:52:17)
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:60:24
    at Root.each (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:41:16)
    at Root.walk (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:52:17)
    at Root.walkDecls (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:70:19)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
$ C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\.bin\vite build -c vite.sdk-lib-config.js

Getting some more debugging info on which variable is failing

Added some extra logging to scripts/postcss/css-compile-variables.js#L78, we can see what declaration it's failing on.

console.log(`resolveDerivedVariable decl.prop=${decl.prop} decl.value=${decl.value} variable=${variable} value=${value} baseVariable=${baseVariable} baseVariables=${JSON.stringify(Array.from(baseVariables.entries()))} aliasMap=${JSON.stringify(Array.from(aliasMap.entries()))} resolvedMap=${JSON.stringify(Array.from(resolvedMap.entries()))}`);

For some reason, background-color-secondary isn't being populated in baseVariables:

$ C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\.bin\vite build -c vite.sdk-assets-config.js
resolveDerivedVariable decl.prop=border decl.value=1px solid var(--background-color-secondary--darker-7) variable=--background-color-secondary--darker-7 value=undefined baseVariable=background-color-secondary
baseVariables=[["size","20px"],["avatar-size","12px"],["usercolor1","#368BD6"],["usercolor2","#AC3BA8"],["usercolor3","#03B381"],["usercolor4","#E64F7A"],["usercolor5","#FF812D"],["usercolor6","#2DC2C5"],["usercolor7","#5C56F5"],["usercolor8","#74D12C"]] 
aliasMap=[["icon-color","background-color-secondary--darker-40"],["light-border","background-color-secondary--darker-5"],["light-text-color","background-color-secondary--darker-55"],["timeline-time-text-color","background-color-secondary--darker-35"],["icon-background","background-color-secondary--darker-7"],["right-panel-text-color","background-color-secondary--darker-35"]] 
resolvedMap=[]

[vite:css] Cannot derive from background-color-secondary because it is neither defined in config nor is it an alias!
file: C:/Users/MLM/Documents/GitHub/element/hydrogen-web/src/platform/web/ui/css/themes/element/theme.css?type=runtime
error during build:
Error: Cannot derive from background-color-secondary because it is neither defined in config nor is it an alias!
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\src\platform\web\ui\css\themes\element\timeline.css:24:5
    at resolveDerivedVariable (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\vite.sdk-assets-config.js:281:19)
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\vite.sdk-assets-config.js:329:36
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:72:18
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:55:18
    at Rule.each (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:41:16)
    at Rule.walk (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:52:17)
    at C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:60:24
    at Root.each (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:41:16)
    at Root.walk (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:52:17)
    at Root.walkDecls (C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\postcss\lib\container.js:70:19)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

For reference, here is how it looks when it works successfully:

Successful run
 C:\Users\MLM\Documents\GitHub\element\hydrogen-web\node_modules\.bin\vite build -c vite.sdk-assets-config.js
resolveDerivedVariable decl.prop=border decl.value=1px solid var(--background-color-secondary--darker-7) variable=--background-color-secondary--darker-7 value=#2D3239 baseVariable=background-color-secondary
baseVariables=[["size","20px"],["avatar-size","12px"],["usercolor1","#368BD6"],["usercolor2","#AC3BA8"],["usercolor3","#03B381"],["usercolor4","#E64F7A"],["usercolor5","#FF812D"],["usercolor6","#2DC2C5"],["usercolor7","#5C56F5"],["usercolor8","#74D12C"],["background-color-primary","#21262b"],["background-color-secondary","#2D3239"],["text-color","#fff"],["accent-color","#03B381"],["error-color","#FF4B55"],["fixed-white","#fff"],["room-badge","#61708b"],["link-color","#238cf5"]]
aliasMap=[["icon-color","background-color-secondary--darker-40"],["light-border","background-color-secondary--darker-5"],["light-text-color","background-color-secondary--darker-55"],["timeline-time-text-color","background-color-secondary--darker-35"],["icon-background","background-color-secondary--darker-7"],["right-panel-text-color","background-color-secondary--darker-35"]]
resolvedMap=[]
...
Generated an empty chunk: "index"

Dev notes

  • while yarn build:sdk; do :; done
  • bash -c "while bash scripts/sdk/build.sh; do :; done"
console.log('css-compile-variables once', count, root.source.input.from, root.toString().split("\n").slice(-100).join("\n"));

Platform details

Windows 10 using Cmder. It spawns a Git Bash window to run the build.sh script.

$ node --version
v16.14.0

$ yarn --version
1.22.18

$ git --version
git version 2.28.0.windows.1
@MadLittleMods MadLittleMods added bug Something isn't working sdk labels Apr 19, 2022
@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Apr 19, 2022

I've narrowed it down to to our appended :root { ... } variables just not showing up in the source for the PostCSS plugin sometimes.

I've confirmed that appendVariablesToCSS runs with all of the expected variables in both the failure and successful cases. But after the rollup load, it just isn't in the source for the PostCSS plugin to see sometimes.

https://github.com/vector-im/hydrogen-web/blob/480c5c1584c43c0f2d1ad82054e33b62a442a730/scripts/build-plugins/rollup-plugin-build-themes.js#L167-L172


To note: It doesn't appear to be another Windows path backslash problem like #721 (comment) even though there are a few more cases in our codebase with that same regex. Some code paths seem to get normalized forward slash paths anyway.

@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Apr 20, 2022

On WindowsWith vite@2.9.0+, all of the CSS files root.source.input.from resolve to C:/Users/MLM/Documents/GitHub/element/hydrogen-web/src/platform/web/ui/css/themes/element/theme.css without the extra query parameters we would see on other platforms:

  • /Users/eric/Documents/github/element/hydrogen-web/src/platform/web/ui/css/themes/element/theme.css?variant=dark&dark=true
  • /Users/eric/Documents/github/element/hydrogen-web/src/platform/web/ui/css/themes/element/theme.css?variant=light
  • /Users/eric/Documents/github/element/hydrogen-web/src/platform/web/ui/css/themes/element/theme.css?type=runtime

So the following check in our PostCSS plugin to skip the ?type=runtime file isn't ever hit and we run always run the plugin over the ?type=runtime file:

https://github.com/vector-im/hydrogen-web/blob/480c5c1584c43c0f2d1ad82054e33b62a442a730/scripts/postcss/css-compile-variables.js#L145-L149

And the reason it works sometimes (flakey), depends on the order that the files are processed. We also have the variable maps declared in the plugin instance scope which are shared across all of the files processed. So if we process one of the ?variant=light or ?variant=dark first, the variables are already defined in the baseVariables map and resolve for the other files which don't have them defined.

https://github.com/vector-im/hydrogen-web/blob/480c5c1584c43c0f2d1ad82054e33b62a442a730/scripts/postcss/css-compile-variables.js#L136-L139

We should move the map variables inside the file processing (Once) scope so they aren't shared across plugin runs. This would remove the flakey behavior as the plugin would fail every time. We should probably also pass them around directly to avoid concurrent parallel processing problems.

Then we also need to address a way to skip and detect ?type=runtime on Windows. Either fix the bug that is stripping the extra query parameter information or make it part of the file name. The query parameters are still intact in the id in the rollup load hook so it's happening sometime after that.

@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Apr 20, 2022

I've traced the problem to Vite at packages/vite/src/node/plugins/css.ts#L789 where it cleans the given id.

This behavior regressed or changed in vite@2.9.0 (via https://github.com/vitejs/vite/pull/7173/files#diff-2cfbd4f4d8c32727cd8e1a561cffbde0b384a3ce0789340440e144f9d64c10f6R760) so technically it's not a problem on the latest Hydrogen master which uses vite@2.6.14 but was a problem because I had a later version of vite@2.9.1 installed for #693.

@MadLittleMods MadLittleMods changed the title PostCSS css-compile-variables is flakey on Windows PostCSS css-compile-variables is flakey with vite@2.9.0+ Apr 20, 2022
@MadLittleMods
Copy link
Contributor Author

Asked in vitejs/vite#7822 whether this cleanUrl(id) behavior is intended and here to stay so we know whether we have to refactor scripts/postcss/css-compile-variables.js#L145-L149 or not.

@MadLittleMods
Copy link
Contributor Author

We should move the map variables inside the file processing (Once) scope so they aren't shared across plugin runs. This would remove the flakey behavior as the plugin would fail every time. We should probably also pass them around directly to avoid concurrent parallel processing problems.

@MidhunSureshR Any concern/interest about our custom PostCSS plugins having a bunch of plugin instance-level variables which are shared across all files being processed? Is this by design? It feels like ideally, we don't want this to be the case.

@MidhunSureshR
Copy link
Contributor

We should move the map variables inside the file processing (Once) scope so they aren't shared across plugin runs. This would remove the flakey behavior as the plugin would fail every time. We should probably also pass them around directly to avoid concurrent parallel processing problems.

@MidhunSureshR Any concern/interest about our custom PostCSS plugins having a bunch of plugin instance-level variables which are shared across all files being processed? Is this by design? It feels like ideally, we don't want this to be the case.

This wasn't really by design. I just didn't want to pass the maps around and didn't really see an immediate issue with the approach. We can definitely bring the scope down for those variables 👍 .

MadLittleMods added a commit that referenced this issue May 5, 2022
@MadLittleMods MadLittleMods self-assigned this May 5, 2022
@MadLittleMods
Copy link
Contributor Author

vite@2.9.8 now includes a fix vitejs/vite#7827 and restores the old functionality of not cleaning the ids before passing to the PostCSS functions so everything works again. The dependency is being updated in #693

@bwindels
Copy link
Contributor

Hey @MidhunSureshR could you still have a look to move the scope of those variables down? I think it's a good thing to do regardless of it not being an immediate problem anymore now.

@MadLittleMods
Copy link
Contributor Author

could you still have a look to move the scope of those variables down? I think it's a good thing to do regardless of it not being an immediate problem anymore now.

Created #745 to track this separately ⏩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sdk
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants