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

Update SSR docs for Next.js 11 #2215

Merged
merged 2 commits into from
Aug 12, 2021
Merged

Update SSR docs for Next.js 11 #2215

merged 2 commits into from
Aug 12, 2021

Conversation

reidbarber
Copy link
Member

@reidbarber reidbarber commented Aug 10, 2021

Closes #2159

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

In Docs, check React Spectrum -> Server Side Rendering -> Next.js. Test with new Next app.

🧢 Your Project:

RSP

@reidbarber reidbarber added documentation Improvements or additions to documentation ssr labels Aug 10, 2021
@reidbarber reidbarber added the small review Easy to review PR label Aug 10, 2021
@adobe-bot
Copy link

Build successful! 🎉

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

just one question but otherwise this reads fine

'@adobe/react-spectrum',
'@spectrum-icons/.*',
'@react-spectrum/.*'
const withTM = require("next-transpile-modules")([
Copy link
Member

Choose a reason for hiding this comment

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

Just for my own knowledge: the star patterns don't work any more here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct; it seems like it is not possible since v5 of this plugin (Source). I believe these docs were written at v4. There are some alternate solutions in the thread, so if people feel strongly about implementing one of those, that could be an option.

Copy link
Member

Choose a reason for hiding this comment

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

You might want to explain what is going on here. Is this just a list of everything you use in your project? Would you really have the monopackage and a bunch of the individual ones too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, I'll add that. I wasn't able to get individual packages working. It looks like it requires every one to be included, as well as the monopackage.

Copy link
Member Author

Choose a reason for hiding this comment

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

Leaving as is for now, as to not cause extra confusion. We can follow up if we manage to get individual packages working.

Comment on lines -77 to -78
For an example of a working Next.js app using React Spectrum, see [this repo](https://github.com/devongovett/rsp-next).

Copy link
Member

Choose a reason for hiding this comment

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

No need to handle for this PR, but would be good to track somewhere that we should update/create a new example repo for a example Next.js app

Copy link
Member

Choose a reason for hiding this comment

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

Should we update my app? I accept PRs :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Even better. I'll make one and we can keep this in.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dannify
Copy link
Member

dannify commented Aug 12, 2021

Follow up to update external app.

@adobe-bot
Copy link

Build successful! 🎉

@dannify dannify merged commit d516424 into main Aug 12, 2021
@dannify dannify deleted the Issue-2159-Nextjs-Docs branch August 12, 2021 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation small review Easy to review PR ssr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update SSR docs for Next.js 11
5 participants