Skip to content

Conversation

@peternandersson
Copy link
Contributor

This ensures that there's no gap between our initialization from useState (during render time) and useEffect (post render) for our variable update to come in, which would be missed by this. The alternative to this is to subscribe during render time, but this makes cleanup unnecessarily tricky.

);
const [workbookVariable, setWorkbookVariable] = useState<WorkbookVariable>();

const isFirstRender = useRef<boolean>(true);
Copy link

Choose a reason for hiding this comment

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

I have a slight preference for a separate useEffect here, all else being equal

Copy link
Contributor Author

@peternandersson peternandersson May 23, 2025

Choose a reason for hiding this comment

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

Sure, I'm happy either way. The issue with that is that we'd have to eslint ignore the deps, but either way works.

@peternandersson peternandersson requested a review from slequar May 23, 2025 23:56
@peternandersson peternandersson merged commit 1b83221 into main May 28, 2025
1 check passed
@peternandersson peternandersson deleted the peter/better-use-variable-fix branch May 28, 2025 20:02
@peternandersson peternandersson restored the peter/better-use-variable-fix branch June 7, 2025 00:46
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.

4 participants