Fix useDynamicTextareaHeight initial render with slots

[iansan5653]

Sometimes, useDynamicTextareaHeight cannot determine what the height of the textarea should be on initial render. This can be seen in Projects in the single-select editor dialog, which is contained inside a Dialog v2 component:

Screen.Recording.2023-04-19.at.3.03.18.PM.mov

Notice how the description field initially has extra height, but then collapses down to the correct height after the user starts typing. This is jarring and annoying.

It appears that this is caused by an interaction with the 'slots' pattern where the input is initially hidden on first render, then becomes visible after the layout effect has already run. The layout effect does not rerun when the slots resolve, causing the height to be incorrectly calculated on first render.

We could update the effect to re-run on every render, but because it sets state this would cause a cyclical infinite render loop. Alternatively, we could change the effect to a regular effect (non-layout) which would cause it to always be correct, but this CSS update in an effect would cause distracting flashes when not using the slots pattern.

So this is a hacky workaround to resolve issues with slots, which is itself a hacky workaround: we run a regular effect in addition to a layout effect on first render, then only layout effects from then on.

[changeset-bot]

🦋 Changeset detected

Latest commit: 1026d29

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[josepmartins]

Het @iansan5653, there are some linting errors with unnecessary dependencies and the UseEffect declaration. Can you take a look at those? 🙏

[github-actions]

size-limit report 📦

Path	Size
dist/browser.esm.js	101.01 KB (0%)
dist/browser.umd.js	101.57 KB (0%)

[iansan5653]

Should be good now!

[josepmartins]

LGMT! I adding @rezrah as a reviewer as well 🙏

[colebemis]

@iansan5653 Is this still necessary with the new implementation of slots? https://github.com/primer/react/blob/main/src/hooks/useSlots.ts

[iansan5653]

Probably not - let's keep an eye on it after we update dotcom and close this out if it resolves itself.

[iansan5653]

@colebemis it looks like we haven't shipped slots updates for Dialog (v2) yet? Maybe this would still be useful to merge to fix the bug there for now - then we can revert it once the old slots is entirely gone.

[joshblack]

bump @colebemis wanted to check in if this was something we still need to include in the release or if it's already been addressed by the slots updates mentioned above 👀

[iansan5653]

😕 Actually I think I was wrong - doesn't look like DialogV2 uses slots. The only slots I can see here are FormControl which already got the update and should be live in dotcom. So maybe we do still need to merge this fix - maybe I was just wrong about it being caused by slots.

[colebemis]

Thanks for your patience! These changes look good to me 👍