Skip to content
This repository has been archived by the owner on Jan 10, 2024. It is now read-only.

fix: check for root #1

Closed
wants to merge 1 commit into from
Closed

fix: check for root #1

wants to merge 1 commit into from

Conversation

mattrothenberg
Copy link

Addresses the Todo item about HMR not working.

I'm not sure I fully understand which of these implementation details we want to hide from end users – is this too in the weeds for the actual block template itself?

Copy link

@jaked jaked left a comment

Choose a reason for hiding this comment

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

Comment on lines +80 to +82
if (root) {
root.render(component);
} else {
Copy link

@jaked jaked Nov 18, 2022

Choose a reason for hiding this comment

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

I think this will never run, since let root is scoped to the function and will be recreated as undefined every run. You could do it with a let outside the function (but I'm not sure what happens to these if the code is reloaded).

Copy link
Author

Choose a reason for hiding this comment

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

Curious, that's not what I was observing on my end.

Closing out this patch in any case!

Copy link

Choose a reason for hiding this comment

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

ok I did not actually test it, you may be right! (@Wattenberger and I ended up stripping out the React code here anyway, not sure if she pushed the work we did Friday)

@mattrothenberg mattrothenberg deleted the mr/set-props-refactor branch November 19, 2022 06:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants