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

feat(pure): add renderOptions support to render #1333

Conversation

naorpeled
Copy link

@naorpeled naorpeled commented Jun 1, 2024

What:

Why:

How:

Checklist:

  • Documentation added to the
    docs site
  • Tests
  • TypeScript definitions updated
  • Ready to be merged

closes 1297

@naorpeled naorpeled marked this pull request as draft June 1, 2024 19:55
Copy link

codesandbox-ci bot commented Jun 1, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit faba7c1:

Sandbox Source
react-testing-library-examples Configuration

@naorpeled naorpeled force-pushed the feat/pure/add-render-options-support branch 5 times, most recently from 3225b66 to d5c1a9c Compare June 1, 2024 21:29
@naorpeled naorpeled force-pushed the feat/pure/add-render-options-support branch from d5c1a9c to faba7c1 Compare June 1, 2024 21:42
@eps1lon
Copy link
Member

eps1lon commented Jun 2, 2024

We should flatten this instead of allowing arbitrary options. We don't know if a passing all through will make sense. If people want to access the root options directly, they can always switch to createRoot directly.

@naorpeled
Copy link
Author

We should flatten this instead of allowing arbitrary options. We don't know if a passing all through will make sense. If people want to access the root options directly, they can always switch to createRoot directly.

how should we handle the cases where the consumer uses the non-canary version that currently does not expose those options?

@eps1lon
Copy link
Member

eps1lon commented Jun 11, 2024

how should we handle the cases where the consumer uses the non-canary version that currently does not expose those options?

Is there something different in that regard between your initial version and the version I proposed?

@naorpeled
Copy link
Author

naorpeled commented Jun 22, 2024

how should we handle the cases where the consumer uses the non-canary version that currently does not expose those options?

Is there something different in that regard between your initial version and the version I proposed?

First of all, sorry for the delayed response.

Afaik it's different in the sense that we base the type definitions on what the version the consumer uses,
in React 18 they won't get type autocompletions for v19 canary definitions, and vice verse.

@eps1lon
Copy link
Member

eps1lon commented Jun 25, 2024

Afaik it's different in the sense that we base the type definitions on what the version the consumer uses,
in React 18 they won't get type autocompletions for v19 canary definitions, and vice verse.

That's intended though, no? I don't see how type concerns need to dictate our API. It seems to me we can make flat options work. It just requires a bit more type-implementation work on our part.

@eps1lon
Copy link
Member

eps1lon commented Aug 28, 2024

Alternate: #1354

@naorpeled naorpeled closed this Aug 28, 2024
@naorpeled
Copy link
Author

naorpeled commented Aug 28, 2024

Alternate: #1354

Sorry I ended up not finishing this task, had some IRL things to deal with.

Thanks for taking this task, I should've communicated earlier.

This pull request was closed.
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