web-infra-dev / rspack

The fast Rust-based web bundler with webpack-compatible API 🦀️
https://rspack.dev
MIT License
9.68k stars 556 forks source link

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

Open Austaras opened 6 days ago

Austaras commented 6 days ago

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 commented 6 days ago

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 commented 6 days ago

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 commented 5 days ago

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

Austaras commented 5 days ago

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

ahabhgk commented 5 days ago

Thanks in advance! You can add tests under packages/rspack-test-tools/tests/configCases, you can refer to configCases/builtins/css-modules-pseudo.

Austaras commented 5 days ago

No I don't believe that would work. configCases only tests if the export mapping is right, but in the actual generated CSS, composes and animation referenced idents are removed.

ahabhgk commented 5 days ago

This should be a better example: configCases/css/rewrite-url-css-variables, you can read the generated css file content, and assert whether the composes and animation referenced idents still exist. (remember use __non_webpack_require__("fs" / "path") and set node.__dirname/__filename to false)