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

Don't generate export for @keyframes #190

Closed
kostaskriaridis opened this issue Mar 9, 2023 · 4 comments
Closed

Don't generate export for @keyframes #190

kostaskriaridis opened this issue Mar 9, 2023 · 4 comments
Labels
wontfix This will not be worked on

Comments

@kostaskriaridis
Copy link

Introduction

Hello, everyone 👋
Thank you for supporting such a great tool 🙏

Expected Behavior

We have the following .scss file:

.root {
  animation: scale-animation 1.4s ease-in-out;
}

@keyframes scale-animation {
  0% {
    transform: scale(0);
  }

  100% {
    transform: scale(1);
  }
}

I expect tsm not to generate named export for @keyframes declaration

Desired output:

export const root: string;

Current Behavior

Actual output:

export const root: string;
export const scaleAnimation: string;

Possible Solution

  1. Not to generate export for @keyframes declaration
  2. Create a separate option for the package like --excludeKeyframes

Context

  1. I enabled namedExport option in css-loader webpack plugin
  2. I enabled import/no-unused-modules eslint rule for .scss.d.ts files
  3. In .scss files, where I have @keyframes declaration, animation name get's generated in .scss.d.ts files and I get an error from import/no-unused-modules, because this variable is not used in my react components

Your Environment

  • Version used: 7.0.2
  • Operating System and versions: macOS Monterey, 12.6.1
  • Link to your project: link
@skovy
Copy link
Owner

skovy commented Mar 14, 2023

Interesting, I think it makes sense to remove keyframes from generation since there's no use case I can think of where you'd want to import them in TS/JS.

The relevant line of code to change for this would be here:

return core.load(source, undefined, undefined, noOpPathFetcher);

We're relying on this package: https://www.npmjs.com/package/css-modules-loader-core

It looks to allow plugins but haven't investigated further. Hopefully we can alter the functionality to filter out keyframes otherwise would need custom logic or consider switching to another package.

@skovy skovy added bug Something isn't working help wanted Looking for help to reproduce, debug, or open a PR labels Mar 14, 2023
@kostaskriaridis
Copy link
Author

Hello, @skovy, thank you for your response

I created a test Pull Request where I use css-tree package for parsing css.

May you please have a look?

@skovy skovy added wontfix This will not be worked on and removed bug Something isn't working help wanted Looking for help to reproduce, debug, or open a PR labels Aug 4, 2023
@skovy
Copy link
Owner

skovy commented Aug 4, 2023

After doing some more research, this is actually intentional and supported behavior. Since someone could be using this and relying on it I don't think this is a bug, but intended functionality.

Example of how this can be used:

@keyframes animate {
  0% {
    transform: scale(0);
  }

  100% {
    transform: scale(1);
  }
}
import styles from "./style.css";

<div style={{ animation: styles.animate, }} />

This allows importing animations by name and applying them dynamically.

@skovy skovy closed this as completed Aug 4, 2023
@jeron-diovis
Copy link

@skovy There is an issue in your example above.
Although css-modules themselves indeed automatically export keyframes along with classnames – but those names are transformed like everything else. So your styles.animate value will contain not an animate string but smth like _animate_vl9dn_43. Which obviously won't work (and makes me question why are they doing this in the first place).

To use it like this you'll need to configure getLocalIdent option of css-loader - and probably follow some naming convention, to be able to recognize keyframes there and don't rename them. I really doubt someone will ever bother with this, when you can just apply animation in a class and export that class as usual.


But regardless, behaviour of typed-scss-module about generating types for keyframes is correct, because it fully matches to how css-modules work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants