-
Notifications
You must be signed in to change notification settings - Fork 7
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
Next.JS compatibility #51
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adams85
reviewed
Mar 22, 2024
|
z4kn4fein
approved these changes
Mar 26, 2024
endret
approved these changes
Mar 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the purpose of your pull request
Using the SDK in Next.JS had issues on the server side - XMLHttpRequest is not defined (#40, #7).
It turned out that simply switching to axios instead of XMLHttpRequest would only hide the original issue but it won't fix the problem.
I added 'use client' to the entry points to indicate that the SDK is intented to be used only on client side.
However it wasn't enough for Next.JS as it prerendered our code on the server side even if I specified the 'use client'.
That would not be a problem by default but we initialized the configcatclient in the ConfigCatProvider's constructor. So when Next.JS prerendered the ConfigCatProvider, it started the polling on the server side as well.
I refactored the code to only initialize the client in componentDidMount. componentDidMount is not running on the server side during the prerendering, so the client will not be initialized on the server side.
Unfortunately I had to introduce a breaking changes to be able to achieve this functionality: from now, the useConfigCatClient hook can return undefined until the Provider is not mounted.
I also created a sample Next.JS app that can use our React SDK.
Related issues (only if applicable)
#40
#7
Requirement checklist (only if applicable)