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

generate-arg-types: Allow multiple exports in component files #1953

Merged
merged 2 commits into from
Jul 18, 2023

Conversation

istarkov
Copy link
Member

@istarkov istarkov commented Jul 18, 2023

Description

Allow multiple exports in components. Most radix components will look like
export const Tooltip = TooltipPrimitive.Root;

The others would be a simpe wrappers over radix with just type changes like like `Omit<TriggerProps, "className">

It's too much to create for each sucj export a standalone file.

After this PR following will work.

import * as TooltipPrimitive from "@radix-ui/react-tooltip";

export const Tooltip = TooltipPrimitive.Root;
export const TooltipTrigger = TooltipPrimitive.Trigger;

Generated exports in case of multiple entries are

export propsTooltip: ...
export propsTooltipTrigger ....

In case of single export previous behaviour is preserved for now, but should be changed IMO in the future PRs

Known bugs

Until this merged we should not use displayName (have no idea why we use it at all) in files with multiple exports
styleguidist/react-docgen-typescript#449

As of now will add comments

import * as TooltipPrimitive from "@radix-ui/react-tooltip";

export const Tooltip = TooltipPrimitive.Root;
// We can't use .displayName until this is merged https://github.com/styleguidist/react-docgen-typescript/pull/449
// Tooltip.displayName = "Tooltip";

export const TooltipTrigger = TooltipPrimitive.Trigger;
// We can't use .displayName until this is merged https://github.com/styleguidist/react-docgen-typescript/pull/449
// TooltipTrigger.displayName = "TooltipTrigger";

Steps for reproduction

  1. click button
  2. expect xyz

Code Review

  • hi @kof, I need you to do
    • conceptual review (architecture, feature-correctness)
    • detailed review (read every line)
    • test it on preview

Before requesting a review

  • made a self-review
  • added inline comments where things may be not obvious (the "why", not "what")

Before merging

  • tested locally and on preview environment (preview dev login: 5de6)
  • updated test cases document
  • added tests
  • if any new env variables are added, added them to .env.example and the builder/env-check.js if mandatory

@vercel
Copy link

vercel bot commented Jul 18, 2023

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

Name Status Preview Updated (UTC)
webstudio-builder ✅ Ready (Inspect) Visit Preview Jul 18, 2023 9:03pm

@istarkov istarkov requested review from kof and TrySound July 18, 2023 20:16
@istarkov istarkov changed the title Allow multiple exports in component files generate-arg-types: Allow multiple exports in component files Jul 18, 2023
@istarkov istarkov merged commit 0459452 into main Jul 18, 2023
@istarkov istarkov deleted the allow-multiple-exports branch July 18, 2023 21:24
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.

3 participants