[RFC] Improving React Developer Experience

[evanSe]

We are on a mission to make the React developer experience next level - Handsontable should be a seamless addition to your React application and we have some great ideas on how to achieve just that.

How can you contribute?

We invite you to share your experiences, pose questions, correct our assumptions, and showcase your unique use cases. Dive into the ideas outlined in the RFC, and embark on this exciting journey with us! 🚀

Goals

Our objectives for the overhaul include:

• Functional Components: rewrite our wrapper in functional components.
• Revamped Editor/Renderer Experience: Bid farewell to the hot-editor and hot-renderer tagging in components. Welcome, render props for a cleaner experience.
• TypeScript Enhancement

Improving the Renderer/Editor experience

Specifying renderers and editors as children components marked with "magic" (hot-renderer, hot-editor etc.) attributes does not feel idiomatic. They don't represent something that is a part of the column, it's purely the column's property. Moreover, this kind of composition allows putting multiple Renderers and Editors in one column, which is illogical.

Instead, we're proposing to use a variant of render props pattern and allow specifying component-based renderers/editors via HotTable or HotColumn props directly. The awesome part of this approach is we can keep things heavily typed in regards to typescript, it is the preferred approach when it comes to proper React and involves less magic behind the scenes.

<HotColumn
	data={1}
	hotRenderer={(props) => (<CustomRenderer {...props} />)}
	hotEditor={(props) => (<CustomEditor {...props} />)}
/>;

Functional components and context

We're planning to get rid of the class-based approach in favor of functional components. The data between HotTable and HotColumns will no longer be passed via component cloning and children manipulation - React context will be used instead.

Currently, we are planning to keep this context private, although there could be some great use cases to expose it, we would love to know!

const HotTable = ({ children }) => {
  return (
    <HotTableContext.Provider value={hot}>{children}</HotTableContext.Provider>
  );
};


const CustomEditor = ({value}: EditorProps) => {
  // perhaps it could be great to have context easily accessible
  const hot = useHot();
  const editor = useEditor();

  const handleValueChange = (event: ChangeEvent<HTMLInputElement>) => {
    const newValue = event.target.value;
    editor.setValue(newValue);
  };

  return <input type="text" value={value} onChange={handleValueChange} />;
};

Other changes we are planning

• We're planning to drop settings prop from HotTable
  • it is an undocumented feature; it creates a dual interface
  • a user can pass things both directly into HotTable props or via settings object. Firstly, it's not clear which one wins in case of conflicts. Secondly, passing objects into props unnecessarily increase the risk of re-renders caused by new object instances being created on every parent render. Thirdly, it creates unnecessary complexity to the internal code.
• We'll make sure all the code is strictly typed with strict TypeScript setting.
• We'll add a check that only HotColumn instances are children of HotTable
• We'll drop unnecessary Redux dependency for typing (it will stay for tests, though).
• We plan to explore cloning issue solutions - possibly using immer (https://immerjs.github.io/immer/) or some kind of similar approach utilising lazy copy idea.

[NOtherDev]

@evanSe I'm thinking about the data mutability issue. In order to make the wrapper actually feel idiomatic in React, we should not only make sure the data isn't mutated, but also make sure the only way to get the changed data will be via onChange prop.

The simple solution could use deep copy of data on init and afterChange to detect changes and construct a modified data object, but I'm pretty sure it will have quite a deep performance impact on a large data sets as deep copies aren't cheap.

I proposed using Immer, but it doesn't seem to be feasible just within the React wrapper as the data mutation happens quite deep in HOT Core and there's no easy way to plug in there and this would probably be too breaking anyway.

How about we use beforeChange hook called from processChanges, note the changes to be applied and cancel them all, so that the original data source isn't modified, and then reconstruct the copy of the data object lazily, Redux way, with just the minimal set of replacements applied?

This will end up not applying the change to the grid unless the user hooks it back via data prop kept in React state, but that's the React way of handling forms anyway, so I think it wouldn't be surprising.

Also, we can think of a "legacy"/"non-managed" mode, for example when someone doesn't provide onChange we can fall back to the current mutating solution. WDYT?

[evanSe]

I have spent some more time thinking about this issue. Currently, I'm inclined not to alter the approach to passing data into the wrapper. Such change would be substantial for users, creating a different experience in React compared to other wrappers and the core library.

It seems that the core of the problem lies within the Handsontable core itself, and any meaningful resolution would require addressing it there first. Attempting to handle this issue without addressing the core could lead to suboptimal outcomes.

I think we shouldn't tackle this change and that our next step should involve preparing the pull request for review by the entire team/community. What do you think?

[NOtherDev]

OK, got it. Yes, if that's the plan to embrace immutability across the whole library, it would definitely makes a little sense to work on this separately here now.

So, seems we're done. I'm going to create feature/imroved-react to develop PR now, although it'll be a nightmare to review 😅

[NOtherDev]

Actually @evanSe , seems I can't create a PR within the upstream branches:

[evanSe]

Actually @evanSe , seems I can't create a PR within the upstream branches:

@NOtherDev you should be able to now

[NOtherDev]

Still the same error @evanSe.
OK, I saw the invite now 🙂