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

Export PatternOptions and VariantGroups types? #694

Closed
igor10k opened this issue May 24, 2022 · 6 comments
Closed

Export PatternOptions and VariantGroups types? #694

igor10k opened this issue May 24, 2022 · 6 comments

Comments

@igor10k
Copy link

igor10k commented May 24, 2022

Could PatternOptions and VariantGroups be exported in @vanilla-extract/recipes to allow creating extendable recipes?

Example:

const createRecipe = <T extends VariantGroups>(
  extension: PatternOptions<T | typeof baseVariants>,
) => {
  return recipe({
    base: extension.base,
    variants: {
      ...baseVariants,
      ...extension.variants,
    },
    compoundVariants: extension.compoundVariants,
    defaultVariants: {
      ...baseDefaultVariants,
      ...extension.defaultVariants,
    },
  });
};

const button = createRecipe<typeof buttonVariants>({
  variants: buttonVariants,
  compoundVariants: [
    // mixing variants from baseVariants and buttonVariants here
  ]
});

There are ways to make it work without them but at the expense of losing auto-suggestions.

@graup
Copy link
Collaborator

graup commented Mar 3, 2023

You could always pull the types out with TS, for example:

import { recipe } from '@vanilla-extract/recipes';

type VariantGroups = NonNullable<Parameters<typeof recipe>[0]['variants']>;
type PatternOptions = Parameters<typeof recipe>[0];

Does this help?

@igor10k
Copy link
Author

igor10k commented Mar 3, 2023

This works but is there any reason not to export them?

@askoufis
Copy link
Contributor

askoufis commented Mar 4, 2023

This works but is there any reason not to export them?

Quoting a reply on discord to a similar question:

michaletaranto: As a general rule, we try to minimise the API footprint as it makes us less vulnerable to breaking changes in the future

@igor10k
Copy link
Author

igor10k commented Mar 9, 2023

I see. Thank for the answers.

@igor10k igor10k closed this as completed Mar 9, 2023
@gabro
Copy link
Contributor

gabro commented May 10, 2023

Hi all, following up on this issue (I can open a new one if preferred), we tried a similar workaround to the one proposed by @graup, but it introduced a huge performance regression when replacing SprinklesProperties with

type VarargParameters<T extends (args: any) => any> = T extends (args: infer P) => any ? P : never;
type SprinklesProperties = VarargParameters<typeof createSprinkles>;

I've ran a CPU profile on the library using this, and it is very clearly pointing at the computation in VarargParameters.

The difference is ~5 seconds (with SprinklesProperties) vs ~2 minutes using the workaround.

Does anyone else face similar issues? I think the culprit here is that Parameters doesn't work with varargs arguments, and it's apparently very slow to roll your own manually.

I'm open to suggestions, but this is currently a blocker for us to upgrade to any newer version of Sprinkles.

@HorusGoul
Copy link

Reopened this discussion with a PR and an argument to make export these types: #1204

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

No branches or pull requests

5 participants