abhaynikam/react-trix-rte

Crash when using ReactTrixRTEToolbar with toolbarActions

Closed this issue · 12 comments

Axxxx0n commented

Because of double peerDependency key in package.json lib crashes when using ReactTrixRTEToolbar with toolbarActions.

https://codesandbox.io/s/sad-leavitt-3s4xq?file=/src/App.js

PR: #23

abhaynikam commented

Closed by #23

alejo4373 commented

I'm running in to this. Any idea when the fix would make it to npm with a new release?

abhaynikam commented

@alejo4373 My bad. Releasing the new version now.

abhaynikam commented

@alejo4373 1.0.11 is released. Could you please help us test if this version fixes the issue? Thanks 😊

  • 👍1
alejo4373 commented

Unfortunately this is still an issue. I reproduced it in this codesandbox. I'm using the latest version of react-trix-rte and trix 😢 . Happy to help where I can. How is the setup of the Storybook demo different?

cc: @abhaynikam @Axxxx0n

abhaynikam commented

@alejo4373 I'll take a look at this today. Thanks for building a sandbox to reproduce the issue.

alejo4373 commented

So I played with it for a bit and I think this is a problem with ramda. I cloned this repo and I ran the example with yarn build-examples after adding the toolbarActions prop and I got the same error. Furthermore inspecting what is imported from ramda wiith a simple console.log reveals that .includes is not a property of the R object. When I manually install the latest version of ramda everything worked just fine.

abhaynikam commented

@alejo4373 Cool. It would be great help if you could raise a patch for it as you are already have found the issue. I'll make sure I update the storybooks examples as well on the deployed site. Thanks 😊

alejo4373 commented

Sure, I'll try to have something tomorrow or before the end of the week. I might have a few questions later on, though.

alejo4373 commented

like should ramda be a direct dependency instead of a peer dependency? I'm new to the peer dependency ordeal

abhaynikam commented

@alejo4373 I think the we should work on removing ramda completely because adding it to dependency increase the bundle size and we do not need Ramda so much. Ramda usage in the lib is very less and can be easily replaced.

The easy fix for the current issue with toolbar is to replace R.includes with toolbarActions.includes("link")

  • 👍1
abhaynikam commented

Thanks @alejo4373 for working on this ❤️ . I think this issue is now resolved and we can close it. Please free to test it on 1.0.13 version.