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

[Bug]: css animation is removed in css module #8211

Open
Austaras opened this issue Oct 24, 2024 · 4 comments
Open

[Bug]: css animation is removed in css module #8211

Austaras opened this issue Oct 24, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@Austaras
Copy link
Contributor

System Info

System:
OS: Linux 6.11 Arch Linux
CPU: (32) x64 13th Gen Intel(R) Core(TM) i9-13980HX
Memory: 44.75 GB / 62.56 GB
Container: Yes
Shell: 3.7.1 - /usr/bin/fish
Binaries:
Node: 22.10.0 - /usr/bin/node
pnpm: 9.12.1 - /usr/bin/pnpm
npmPackages:
@rspack/cli: ^1.0.14 => 1.0.14
@rspack/core: ^1.0.14 => 1.0.14

Details

css classes is preserved, but @keyframes is removed.

Reproduce link

https://github.com/Austaras/rspack-css-module

Reproduce Steps

see dist folder

@Austaras Austaras added bug Something isn't working pending triage The issue/PR is currently untouched. labels Oct 24, 2024
@Austaras
Copy link
Contributor Author

After some investigation, this bug is caused by not adding animation names to the deps of exported name, which is impossible to fix because css-module-lexer doesn't expose local classes to keyframe deps, @ahabhgk would you rather fix it yourself or me sending a PR?

@ahabhgk
Copy link
Collaborator

ahabhgk commented Oct 24, 2024

I see, the keyframe dependency (the aaa inside .foo local class) is actually depend on the keyframe decl dependency (the @keyframe aaa), so for this css module, there is a kind of self reference we didn't handle well. And since the keyframe decl is actually depended by the keyframe, not the local class, so we don't need to change css-module-lexer.

I think we can fix this by adding get_referenced_exports for the keyframe dependency, then rspack's treeshaking will flag the keyframe decl dependency as "used", and won't remove it from the output asset. For now in rspack keyframe and keyframe decl are both using CssLocalIdentDependency, we probably need to create CssUseLocalIdentDependency for the keyframe. Do you want to send a PR?

@Austaras
Copy link
Contributor Author

Sure. But I would require some assistance. First, how to add a proper test?

@Austaras
Copy link
Contributor Author

Well I thought css module compose works, but it doesn't either. You may see the new commit in my repro

@jerrykingxyz jerrykingxyz removed the pending triage The issue/PR is currently untouched. label Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants