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

Experimental getSchema #333

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Experimental getSchema #333

wants to merge 10 commits into from

Conversation

joe-bell
Copy link
Owner

@joe-bell joe-bell commented Jan 23, 2025

Warning

Extremely WIP

Description

Surfaces a getSchema function to extract a cva component's variant schema for documentation purposes

Inspired by @bengry's wonderful proposal

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Follow the Style Guide.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).

Copy link

vercel bot commented Jan 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Jan 25, 2025 6:19pm
docs-beta ✅ Ready (Inspect) Visit Preview Jan 25, 2025 6:19pm

@bengry
Copy link

bengry commented Jan 23, 2025

@joe-bell Just a heads up, if it helps - my fork already has some tests for .variants being exposed (as well as the other stuff it adds). They're not exhaustive, but feel free to take inspiration from them.

Overall the code in both the actual code as well as the tests are very similar to the current state of [email protected], so you can probably even compare the two branches to save you some time.

@joe-bell
Copy link
Owner Author

joe-bell commented Jan 24, 2025

Thanks @bengry!

I think my preference here would be for us to instead do something kinda like:

const getSchema: GetSchema = (comp) => {
  // CVA function would spit the `variants` out from the specified `config`– untouched – into a hidden `_cva` metadata object. Which we'd use to create the schema
  return comp._cva?.variants
    ? Object.fromEntries(
        Object.entries(comp._cva?.variants).map(([key, value]) => [
          key,
          Object.keys(value).map((propertyKey) =>
            normalizeVariantKey(propertyKey),
          ),
        ]),
      )
    : {};
};

// Demo
const button = cva({
    variants: {
      size: {
        sm: "p-2",
      },
    },
  });

const schema = getSchema(button); // { size: readonly "sm"[]; }

I'd largely consider getting the schema to be somewhat of a power-user feature and I wouldn't want to inflate the cva much further if possible – especially considering there's some more features I'm considering

Biggest challenge is going to be typing these damn types – it's had me pretty stumped today

@joe-bell
Copy link
Owner Author

Making good progress, but gotta whole lotta types nonsense to figure out here

@@ -35,7 +35,7 @@
"build:cjs": "swc ./src/index.ts --config-file ./.config/.swcrc -o dist/index.js -C module.type=commonjs",
"build:esm": "swc ./src/index.ts --config-file ./.config/.swcrc -o dist/index.mjs -C module.type=es6 ",
"build:tsc": "tsc --project .config/tsconfig.build.json",
"bundlesize": "pnpm build && bundlesize -f 'dist/*.js' -s 1.6KB",
"bundlesize": "pnpm build && bundlesize -f 'dist/*.js' -s 2KB",
Copy link
Owner Author

@joe-bell joe-bell Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporary bump. Might consider moving getSchema into a separate cva/utils entry point to keep bundlesize lower

@bengry
Copy link

bengry commented Jan 25, 2025

@joe-bell I didn't look at your implementation too much, but here's a very quick POC I made for what you suggested. It split the ~2kB bundle size into two parts, and using ESM further reduces the bundle size a bit:

PASS  ./dist/index.mjs: 962B < maxSize 1KB (gzip) 
PASS  ./dist/utils/index.mjs: 410B < maxSize 500B (gzip) 

I also replaced swc with tsup since it basically does the same thing with less config IMO, and I couldn't get swc to bundle multiple entrypoints easily. unbuild might also be worth a look, though I haven't looked at it too much.

Basic types work as expected, but I didn't implement the private variants, or the same for compose. There's also some copy-paste stuff, and circular type dependencies going on which need to be cleaned up. Should be easy enough to do.

@joe-bell
Copy link
Owner Author

Oh nice, thanks for the heads up @bengry, I'll take a look!

Feel pretty happy with the progress on my solution so far –  mostly type issues

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

Successfully merging this pull request may close these issues.

2 participants