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.

Sign up for GitHub

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

Jump to bottom

Portal is not ssr compat - accesses document during render #3721

Open
mattcosta7 opened this issue Sep 8, 2023 · 7 comments · May be fixed by #5635
Open

Portal is not ssr compat - accesses document during render #3721

mattcosta7 opened this issue Sep 8, 2023 · 7 comments · May be fixed by #5635
Labels
bug Something isn't working react

Comments

@mattcosta7
Copy link
Collaborator

mattcosta7 commented Sep 8, 2023 •
edited
Loading

Description

When using InlineAutocomplete we inherit usage of <Portal /> rendering on page load.

I think the issue occurs because <Portal> calls document.createElement and createPortal with a ref to an element that can't be created.

react/src/Portal/Portal.tsx

Line 68 in 500e529

const hostElement = document.createElement('div')

we should avoid these apis in render in portal (which creates a div on every render) and rely on other mechanisms to sync it

In addition to this, it renders an _Autocomplette which uses useCombobox which calls useLayoutEffect resulting in a call to console.error on the server -

react/src/drafts/hooks/useCombobox.ts

Lines 141 to 157 in 500e529

useLayoutEffect(() => {
const optionElements = getOptionElements()
// Ensure each option has a unique ID (required by the Combobox class), but respect user provided IDs
for (const [i, option] of optionElements.entries()) {
if (!option.id || option.id.startsWith(optionIdPrefix)) option.id = `${optionIdPrefix}-${i}`
option.setAttribute('data-combobox-list-index', i.toString())
option.addEventListener('mousedown', onOptionMouseDown)
// the combobox class has a bug where it resets the default on navigate, but not on clearSelection
option.removeAttribute('data-combobox-option-default')
}
comboboxInstance?.clearSelection()
return () => {
for (const option of optionElements) option.removeEventListener('mousedown', onOptionMouseDown)
}
}, [getOptionElements, optionIdPrefix, options, comboboxInstance, onOptionMouseDown])

Steps to reproduce

Atempt to SSR an app that uses InlineAutocomplete

Version

latest

Browser

No response
@mattcosta7 mattcosta7 added the bug Something isn't working label Sep 8, 2023
@mattcosta7 mattcosta7 changed the title Portal uses useLayoutEffect which isn't ssr compat Portal is not ssr compat Sep 8, 2023
@mattcosta7 mattcosta7 changed the title Portal is not ssr compat Portal is not ssr compat - accesses document during render Sep 8, 2023
This was referenced Sep 8, 2023
Merged
Closed
@lesliecdubs
Copy link
Member

lesliecdubs commented Sep 11, 2023 •
edited
Loading

@mattcosta7 thanks for filing and for putting up some PRs! Are you still looking into a solve, or would you like support? If the latter, can you also give us some info on whether this is blocking a particular product feature?

@mattcosta7
Copy link
Collaborator Author

@mattcosta7 thanks for filing and for putting up some PRs! Are you still looking into a solve, or would you like support? If the latter, can you also give us some info on whether this is blocking a particular product feature?

I don't think I'll have time , but I don't believe it's blocking anything!

@lesliecdubs
Copy link
Member

Thanks, copy that. We'll move this to the backlog for now, but happy to reassess priority at any point as needed.

@github-actions GitHub Actions
Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 10, 2024
@mattcosta7
Copy link
Collaborator Author

not stale

@mattcosta7 mattcosta7 removed the Stale label Mar 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 7, 2024

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 7, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 14, 2024
@mattcosta7 mattcosta7 reopened this Jan 31, 2025
@github-actions github-actions bot removed the Stale label Jan 31, 2025
@joshblack joshblack linked a pull request Jan 31, 2025 that will close this issue
Draft
13 tasks
@keithamus
Copy link
Member

Looks like I haven't done my usual "we should use popover instead" so:

Rather than continuing to invest in portals we should be migrating the remaining cases to use popover instead. There is no good reason to use portals now that popover exists.

@keithamus keithamus marked this as a duplicate of #5647 Feb 3, 2025
@keithamus keithamus mentioned this issue Feb 3, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working react
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(Portal): update portal to be SSR compatible primer/react
3 participants
@keithamus @lesliecdubs @mattcosta7