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

Preact: Pass component instead of props to Meta type #28472

Open
wants to merge 10 commits into
base: next
Choose a base branch
from

Conversation

NotWoods
Copy link
Contributor

@NotWoods NotWoods commented Jul 6, 2024

Closes #21766

What I did

The Preact renderer is missing features from the React renderer, such as supporting Meta<typeof Component> instead of Meta<ComponentProps>. This PR updates the Preact renderer to be as close as possible to the React renderer source code (sometimes moving lines around) to support all the same TypeScript features. This also includes adding tests, which wasn't present in the package previously.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Run yarn task check to validate typing changes

Documentation

TODO: should update docs, porting samples from React

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Updated code/renderers/preact/package.json with new dependencies for TypeScript features.
  • Added code/renderers/preact/src/__test__/Button.stories.tsx for Button component stories in CSF2 and CSF3 formats.
  • Introduced code/renderers/preact/src/__test__/Button.tsx with a new Button component.
  • Modified code/renderers/preact/src/entry-preview.ts to reorder exports and add renderToCanvas.
  • Updated code/renderers/preact/src/render.tsx to simplify rendering and remove StoryHarness.

12 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings

code/renderers/preact/src/__test__/Button.tsx Outdated Show resolved Hide resolved
code/renderers/preact/src/__test__/Button.tsx Show resolved Hide resolved
Copy link

nx-cloud bot commented Jul 6, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b5fca27. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@shilman shilman changed the title feature request: Pass component instead of props to preact Meta (like React) Preact: Pass component instead of props to Meta type Jul 15, 2024
type AddMocks<TArgs, DefaultArgs> = Simplify<{
[T in keyof TArgs]: T extends keyof DefaultArgs
? // eslint-disable-next-line @typescript-eslint/ban-types
DefaultArgs[T] extends (...args: any) => any & { mock: {} } // allow any function with a mock object
Copy link
Contributor

@kasperpeulen kasperpeulen Jul 17, 2024

Choose a reason for hiding this comment

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

Can you explain about the mock use case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pretty much just copied React's types here. I assume this is for @storybook/test's fn() result.

@kasperpeulen kasperpeulen added the ci:daily Run the CI jobs that normally run in the daily job. label Jul 17, 2024
@kasperpeulen
Copy link
Contributor

kasperpeulen commented Jul 17, 2024

Could you resolve the merge conflicts @NotWoods? Running CI for this, looks good!

@kasperpeulen
Copy link
Contributor

Could you check the build failures?
https://cloud.nx.app/runs/DkPd02H4j8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ci:daily Run the CI jobs that normally run in the daily job. ci:normal preact typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: New 4.9 types are not working for Preact
3 participants