onInject, onDispose

[pelikhan]

In retrospect, these 2 callback can be replaced with useEffect

   const { workspace ...
    useEffect(() => {
        // onInject    
        return () => // onDispose
    }, [workspace])

react-blockly/src/useBlocklyWorkspace.js

Line 25 in bdf8bde

onInject,

[nbudin]

Hmm. I see your point but I think it's still potentially useful to expose these callbacks, because they're a lot easier to use - particularly onDispose, since otherwise you would have to keep track of the workspace instance yourself. Here's a comparison:

function MyComponent() {
  useBlocklyWorkspace({
    ...,
    onInject: (workspace) => { installPlugin(workspace); },
    onDispose: (workspace) => { removePlugin(workspace); },
  });
}

vs.

function MyComponent() {
  const { workspace } = useBlocklyWorkspace({ ... });
  const [prevWorkspace, setPrevWorkspace] = useState();

  React.useEffect(() => {
    if (prevWorkspace) {
      removePlugin(prevWorkspace);
    }

    setPrevWorkspace(workspace);
    installPlugin(workspace);    
  }, [workspace]);
}

From my point of view, the onInject/onDispose version is much cleaner to read and easier to write. I do agree it's not strictly necessary but I think it's a nice convenience for users.

[pelikhan]

Actually the useEffect callback captures the workspace and you return the cleanup function from the useEffect callback.
Smaller is better, your call.

function MyComponent() {
  const { workspace } = useBlocklyWorkspace({ ... });
  useEffect(() => {
        if (workspace)  installPlugin(workspace);
        return () => workspace && removePlugin(workspace);
    }
  }, [workspace]);
}

[nbudin]

Oh, gotcha - that's a good point. In that case, yeah, I think I'm ok with removing those callbacks and instead adding that example to the documentation.

[pelikhan]

Tested as working on my side :)

[pelikhan]

Have you given a tough about enabling discussions here?

[nbudin]

Thanks for testing!

Re discussions, I haven't really thought about it very much. I'm a little wary of it since it seems like it would encourage people to ask questions, and as I said in the readme I haven't really got a lot of time for this project anymore since I no longer work at a place that uses it. If I were to enable discussions on here, I think I would want to have volunteers who have agreed to help answer questions there. I'd help as much as I can, of course, I just want to set proper expectations about it.