Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Support React Context API #40

Merged
merged 18 commits into from
Jul 22, 2022
Merged

Support React Context API #40

merged 18 commits into from
Jul 22, 2022

Conversation

cbravobernal
Copy link
Collaborator

@cbravobernal cbravobernal commented Jul 4, 2022

Fixes #38

@luisherranz
Copy link
Member

luisherranz commented Jul 5, 2022

Initial sync is working, but there are some remaining tasks:

  • Make sure subscribers map is only created once
  • Subscribe to block + Context, not only to Context (maybe we could use this)
  • Sync initial value as well
  • Check if the React warning of updating Provider during the Consumer rendering (fixed with the setTimeout at the moment) can be avoided
  • Unsubscribe on the useEffect return
  • Add maps or forEach in usesContext and providesContext arrays
  • Investigate if using WeakMap instead of Map makes sense to avoid memory leaks

Finally, it'd be good to start thinking about how React Context should be used in the Editor.

@cbravobernal
Copy link
Collaborator Author

I'm not a memory leaks expert but, with the debug tool, it seems that using WeakMap is a better option, maybe due to its garbage collection behavior.

Without Weakmaps:
without_weakmaps

With Weakmaps
with_weakmaps

@cbravobernal
Copy link
Collaborator Author

Now the initial value is synced, by using a similar approach as the subscriber.

@cbravobernal
Copy link
Collaborator Author

cbravobernal commented Jul 14, 2022

If we have more than one Context in view.js files, it seems that, in the Children element, we only create the Provider the last item of the array. The code causing this issue seems to be around line 145.

In the screenshot, we can see that, on the parent component, we have CounterContext provider and ThemeContext provider, while in the child component, we only have the Counter Context.

Screenshot 2022-07-15 at 17 57 58

I did a commit so anyone can test it and try to find why it happens.

@luisherranz luisherranz changed the title WIP: Add React Context API between blocks. Support React Context API Jul 22, 2022
@luisherranz luisherranz self-assigned this Jul 22, 2022
@luisherranz luisherranz marked this pull request as ready for review July 22, 2022 14:59
@luisherranz
Copy link
Member

This is ready now.

I've done a big refactoring of frontend.js because it was getting a bit out of hand. There will be merge conflicts in #28, but I think it was worth it.

I've also removed dprint because it was failing, and I didn't know why and I wanted to remove it anyway.

@luisherranz luisherranz merged commit be4af26 into main Jul 22, 2022
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.

Support React's Context API between different blocks
2 participants