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

Fix Document editor with "use client" for getContext in NextJS App Directory #8403

Merged
merged 3 commits into from
Mar 29, 2023

Conversation

borisno2
Copy link
Member

@borisno2 borisno2 commented Mar 21, 2023

Currently, if you add the document field to a project and use getContext in the NextJs app directory, the next build will fail due to how the document editor validates the schema on the server side - the result being useState in a server-side component. This PR marks two files with "use client" so Next knows these are client-side files.

Thanks to @gautamsi for working this one out - see comment on #8371

@changeset-bot

This comment was marked as resolved.

Copy link
Member

@dcousens dcousens left a comment

Choose a reason for hiding this comment

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

LGTM, though I wonder how scalable this is for nextjs@13 if you need upstream libraries to add directives

@borisno2
Copy link
Member Author

This is failing due to preconstruct and rollup bundling files together where Module Level directives are ignored. That makes this approach not achievable.

The root cause of this is the crossover of React/Next code in the getContext - in the document field case, this is used for validation of the JSON object to stop runtime errors in the frontend (Keystone effectively renders the Document Editor on the backend to parse and validate the JSON). Ideally, the root cause of this needs to be resolved - remove the need for and decouple the Admin UI (Studio) from the API framework (getContext) this will also help with reducing the weight of keystone when using getContext

Closing this PR, while we continue to explore this use case in #8371

@borisno2 borisno2 closed this Mar 22, 2023
@borisno2
Copy link
Member Author

Giving this another go in light of the latest preconstruct release #8423

@borisno2 borisno2 reopened this Mar 28, 2023
@borisno2 borisno2 force-pushed the document-editor-use-client branch from 67b3875 to fc6e712 Compare March 28, 2023 03:25
@codesandbox-ci

This comment was marked as resolved.

@gautamsi
Copy link
Member

great to see this back, please release soon :)

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.

3 participants