react icon indicating copy to clipboard operation
react copied to clipboard

Figma connect files

Open lukasoppermann opened this issue 1 year ago • 2 comments

Closes #

Changelog

New

Changed

Removed

Rollout strategy

  • [ ] Patch release
  • [ ] Minor release
  • [ ] Major release; if selected, include a written rollout or migration plan
  • [ ] None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

  • [ ] Added/updated tests
  • [ ] Added/updated documentation
  • [ ] Added/updated previews (Storybook)
  • [ ] Changes are SSR compatible
  • [ ] Tested in Chrome
  • [ ] Tested in Firefox
  • [ ] Tested in Safari
  • [ ] Tested in Edge
  • [ ] (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

lukasoppermann avatar Jun 18 '24 13:06 lukasoppermann

⚠️ No Changeset found

Latest commit: 2bc6b9eea13e415d0e3f27046b784ef30ed51398

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Jun 18 '24 13:06 changeset-bot[bot]

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 91.07 KB (-0.01% 🔽)
packages/react/dist/browser.umd.js 91.31 KB (-0.01% 🔽)

github-actions[bot] avatar Jun 18 '24 13:06 github-actions[bot]

Hey @joshblack I think we need to disable the type check for those files, or do you know how we can fix it? We can't add an as TYPE statement, as this will me transferred to figma.

lukasoppermann avatar Jul 02 '24 15:07 lukasoppermann

@lukasoppermann just pushed up a commit to get TypeScript ✅ (hope that's okay, let me know if not!)

Let me know if that matches what you were thinking for these components or not 👀

joshblack avatar Jul 02 '24 17:07 joshblack

@lukasoppermann I'll revert the commit based on your comments 👍 I'll add more info here for where the changes came from.

This has to be reverted, we need children here.

The type error is because counter only accepts string | number in the component but we're giving it a JSX.Element from figma.children.

We need double quotes, otherwise we end up with no quotes in figma.

The type error here is that the value being passed on as current is not 'page' but instead is '"page"' with the nested double quotes.

this does not work I think as the code is not executed. But I'll check.

The type error here is because leadingVisual does not accept JSX.Element, it expects a React.FunctionComponent which is where () => leadingVisual comes from.


My assumption is that we're running into an issue between how we want to represent it in Figma and how it's structured over in TS world 👀 Let me know how you would like to resolve these. My fear for disabling TypeScript on these examples is that TS seems to be catching snippets that would be invalid if someone tried to copy and use them in code due to the type mismatches.

joshblack avatar Jul 02 '24 17:07 joshblack