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

feat: Add {file, name, dir, ext} to classNameSlug resolution #825

Merged
merged 1 commit into from Aug 30, 2021
Merged

feat: Add {file, name, dir, ext} to classNameSlug resolution #825

merged 1 commit into from Aug 30, 2021

Conversation

mikestopcontinues
Copy link
Contributor

Fixes #650
Fixes #571

Motivation

If you want to know where a particular class is coming from, you need more than a hash or even the original title of the class. How many .inner or .title classes have you seen in a large codebase, right?

Summary

This PR adds four new vars to classNameSlug resolution. It also adds the original two hash and title to the object, with the idea that a future breaking change might sandwich the function parameters down to just this one.

While just file would have been sufficient for function style, I added dir, name, ext for ease of use with string-style. Here's what you would expect in each:

{
  hash: 'qdegrke',
  title: 'displayName',
  file: 'this/is/your/module/Component/SubComp.tsx',
  name: 'SubComp',
  ext: 'tsx',
  dir: 'Component',
}

Test plan

Added tests.

@mikestopcontinues
Copy link
Contributor Author

I don't think the circleci issue has to do with my code. I couldn't get it to run properly on my machine either, but yarn watch and yarn test both work.

@Anber
Copy link
Collaborator

Anber commented Aug 30, 2021

Hi @mikestopcontinues! Thank you for your PR.

I don't think the circleci issue has to do with my code. I couldn't get it to run properly on my machine either, but yarn watch and yarn test both work.

There is a problem with types in new code:

src/visitors/GenerateClassNames.ts(150,59): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'ClassNameSlugVars'.
  No index signature with a parameter of type 'string' was found on type 'ClassNameSlugVars'.

You can either relax the definition of ClassNameSlugVars by changing it to Record<string, string> or replace v in slugVars with some more strict type guard (like const isSlugVar = (v: string): v is keyof ClassNameSlugVars => v in slugVars).

All checks can be run by yarn prepare

@callstack-bot
Copy link

Hey @mikestopcontinues, thank you for your pull request 🤗.
The coverage report for this branch can be viewed here.

@mikestopcontinues
Copy link
Contributor Author

@Anber Sorry about that. I just assumed what the circleci issue was. I added one additional test for non-existing replacement patterns to make sure the logic remained consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants