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

RSC-specific workarounds #2050

Merged
merged 4 commits into from Jul 29, 2023
Merged

RSC-specific workarounds #2050

merged 4 commits into from Jul 29, 2023

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented Jul 4, 2023

  • use {} as ReactReduxContext in environments where no context exists
  • switch React imports to namespace imports to prevent hook import detection

* use `{}` as ReactReduxContext in environments where no context exists
* switch React imports to namespace imports to prevent hook import detection
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 4, 2023

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 9a02b4c:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

@phryneas phryneas changed the title map context instances using createContext as key RSC-specific workarounds Jul 4, 2023
@markerikson
Copy link
Contributor

markerikson commented Jul 4, 2023

Is that import switch going to cause any kind of bundle size issue?

(Although... I guess React doesn't get tree shaken anyway...)

@phryneas
Copy link
Member Author

phryneas commented Jul 5, 2023

(Although... I guess React doesn't get tree shaken anyway...)

Pretty much this. All it does is slightly increase the size of the cjs build because it now has to add a namespace import helper function, but no bigger bundle size changes beyond that.

@phryneas phryneas changed the base branch from pr/mapContextByCreateContext to master July 26, 2023 21:43
@EskiMojo14 EskiMojo14 self-requested a review July 26, 2023 21:46
}

function getContext(): Context<ReactReduxContextValue> {
if (!React.createContext) return {} as any
Copy link
Member Author

@phryneas phryneas Jul 26, 2023

Choose a reason for hiding this comment

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

In an environment where createContext does not exist we can pretty much just return anything - there is no useContext that could consume it anyways - and this avoids a hard error being thrown futher down.

Copy link
Collaborator

Choose a reason for hiding this comment

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

and the provider needs to be in a client component, so presumably that won't be an issue either

@phryneas
Copy link
Member Author

I just cherrypicked this commit b37e57e in here since I had forgotten to merge that over earlier. @EskiMojo14 could you take another look please?

@EskiMojo14
Copy link
Collaborator

yep, still lgtm

@phryneas phryneas merged commit 2ac527b into master Jul 29, 2023
13 checks passed
@phryneas phryneas deleted the pr/rsc branch July 29, 2023 15:52
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.

None yet

3 participants