Islands can no longer be “client-side only”#8948
Conversation
mxdvl
force-pushed
the
oj/islands/never-client-side-only
branch
from
7ed39ac to
3da8655
Compare
|
Size Change: 0 B 🆕 Total Size: 0 B |
mxdvl
force-pushed
the
oj/islands/never-client-side-only
branch
4 times, most recently
from
7676892 to
4165ec4
Compare
|
While I agree this is probably the best way to go (since components exist outside their use in an island, it can be unclear when one is intended to be client only vs not client only, etc) - I think we ought to be providing some kind of guidance to developers to help them write islands. This could be as simple as providing an easy component & a util isServer() function - but my preference would be to provide an easy ways to see your component side-by-side in the server render vs the client render (e.g imagine a storybook like interface where you see two versions of your component, one server-side rendered with no JS / hydration, and another client side hydrated version of that same component). I think this would really do a good job to give engineers visibility over the "lifecycle" of the code they're writing. It's worth keeping in mind Islands are quite 'custom' and that a lot of engineers joining the guardian aren't necessarily familiar with server side rendered react or partial hydration models, so we ought to be providing a really well guided experience to writing code in this paradigm. |
mxdvl
force-pushed
the
oj/islands/never-client-side-only
branch
from
4165ec4 to
f9b410d
Compare
Do you see the lack of documentation as a blocker to this change? I think that if anything, this change makes it easier to come up with consistent documentation. None of the proposed changes invalidate existing guidance: dotcom-rendering/dotcom-rendering/docs/contributing/how-to.md Lines 40 to 52 in 7fe3c9a I also think that documentation would be best adressed by the @guardian/dotcom-platform team, who is the more usual steward/custodian of this codebase. |
mxdvl
force-pushed
the
oj/islands/never-client-side-only
branch
from
f9b410d to
005acbc
Compare
Components that rely on accessing `window` or `document` should handle this internally, rather than relying on a property of their parent Island. The behaviour becomes more explicit and less prone to errors. Co-authored-by: Alex Sanders <[email protected]>
mxdvl
force-pushed
the
oj/islands/never-client-side-only
branch
from
005acbc to
ff451af
Compare
| /** | ||
| * This is a weird way of handling this client-only island, | ||
| * but as this component is temporary, it was deemed | ||
| * a more pragmatic approach over having to rewrite it entirely. | ||
| * | ||
| * (October 2023) | ||
| */ | ||
| export const EuropeLandingModal = (props: Props) => | ||
| isServer ? null : <ClientOnlyEuropeLandingModal {...props} />; |
There was a problem hiding this comment.
Discussed with @oliverabrahams & @OllysCoding – this Island cannot run on the server.
individual components can choose to opt out of hydration via the useHydrated hook.
mxdvl
force-pushed
the
oj/islands/never-client-side-only
branch
from
80a6c46 to
3231a33
Compare
4 participants
What does this change?
Remove the
clientOnlyprop fromIsland, forcing components that may be rendered on the server not to throw errors.Why?
Components that rely on accessing
windowordocumentshould handle this internally, rather than relying on a property of their parent Island.The behaviour becomes more explicit and less prone to errors.
Follow-up on #8944
Screenshots
N/A – identical