Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for removing recompose dependency #884

Closed
11 tasks done
stephent opened this issue Feb 28, 2020 · 37 comments · Fixed by #2577
Closed
11 tasks done

Tracking issue for removing recompose dependency #884

stephent opened this issue Feb 28, 2020 · 37 comments · Fixed by #2577
Assignees
Labels

Comments

@stephent
Copy link

stephent commented Feb 28, 2020

Updating React now leads to a deprecation warning in the console:

react.development.js:315 Warning: React.createFactory() is deprecated and will be removed in a future major release. Consider using JSX or use React.createElement() directly instead.
    printWarning @ react.development.js:315
    warn @ react.development.js:278
    createFactoryWithValidation @ react.development.js:1827
    (anonymous) @ shouldUpdate.js:18
    pure @ pure.js:22
    (anonymous) @ compose.js:13
    enhance @ nivo-voronoi.esm.js:82
    ./node_modules/@nivo/voronoi/dist/nivo-voronoi.esm.js @ nivo-voronoi.esm.js:170

This is due to the recompose package which is still used for some of the nivo packages, we have to migrate the following packages to React hooks (providing the same features) to get rid of it:

@codler
Copy link

codler commented Mar 2, 2020

Im getting same warning

@abhaykumar01234
Copy link

Getting the same issue, How to remove the error

@hichana
Copy link

hichana commented Mar 24, 2020

Same issue :(

@neverdane
Copy link

Same here :(

@jungans
Copy link

jungans commented Mar 25, 2020

Same...

@mattcarlotta
Copy link

mattcarlotta commented Mar 26, 2020

This isn't a direct problem with the nivo packages, but instead of with one of its dependencies: recompose.

Specifically recompose/shouldUpdate: https://github.com/acdlite/recompose/blob/master/src/packages/recompose/shouldUpdate.js#L6

and recompose/withPropsOnChange: https://github.com/acdlite/recompose/blob/master/src/packages/recompose/withPropsOnChange.js#L9

Unfortunately, the recompose package appears to be mostly unmaintained. It's recommended to replace this package with some hooks.

@codler
Copy link

codler commented Mar 26, 2020

Reading recompose readme, it suggest you use React Hooks instead

@AmmarHSufyan
Copy link

Same

@jungans
Copy link

jungans commented Apr 9, 2020

There is a PR fixing this issue at recompose repository waiting to be merged for over a month acdlite/recompose#795

@nigellima
Copy link
Contributor

Any updates on this? It's still showing in version 0.61.1

@ElderLK
Copy link

ElderLK commented May 18, 2020

It's still showing to me too. Warning: React.createFactory() is deprecated. I am still waiting for a solution.

@wyze
Copy link
Contributor

wyze commented May 18, 2020

The solution from our end is to move away from recompose as a dependency. Which some charts already are using hooks instead. There are still a bunch of charts using this method, so it will take some time to get them all converted.

If anyone is interested in converting one, please don't hesitate to open a PR. Here is an example of a commit where this was done: 51a58c1

@IzioDev
Copy link

IzioDev commented Jul 31, 2020

Can someone make a list of the components that need a move away from recompose?

@wyze
Copy link
Contributor

wyze commented Jul 31, 2020

Packages list moved to main description.

@shezzor
Copy link

shezzor commented Oct 7, 2020

recompose now also has a dos security flaw being flagged with its deep dependency on node-fetch.

#1129

@brianespinosa
Copy link

I am willing to help contribute getting some of the list that @wyze mentions above converted over to a hooks-based implementation... but I do not want to invest that time if we have no chance of getting that work merged/published. The last published NPM release for Nivo was in 2017.

@plouc if we get this work done to where we can drop the recompose dependency, will we be able to get a new package published?

Yes, I am aware that we could declare the dependency in package.json as the forked and updated repo... I would rather not do that, and I do not want to be responsible for anyone who ends up pointing to that fork.

@plouc
Copy link
Owner

plouc commented Oct 14, 2020

@brianespinosa while I agree I haven't been really reactive lately due to personal constrains, the latest release is from 4 months ago, I guess you referring to the old nivo package VS the scoped ones @nivo/x.

I understand that it can be quite frustrating to contribute on projects and to get no feedback or release including your work (I've experienced it too), and I really apologize for this.

I'll try to release something as soon as I can, but I prefer to not give a date, as each time I did, I had other priorities and could not release.

Unfortunately, forking would not work, you cannot point to repo packages directly as they have to be compiled, and the generated artifacts are ignored, also, as you mentioned, this approach has drawbacks and I would definitely not recommend doing this even if that was feasible.

@plouc
Copy link
Owner

plouc commented Oct 14, 2020

@brianespinosa, I'm currently working on the state of CSS/JS surveys and I usually spend some time on nivo too as they use a lot of dataviz, so this should happen soon enough.

@brianespinosa
Copy link

Thanks @plouc

In this case, I am going to let some of my team know that we can target some of these charts for refactoring. I can report back in about a week with where some of my team want to focus their efforts so we can coordinate.

@plouc
Copy link
Owner

plouc commented Nov 6, 2020

Now removed from @nivo/pie.

@anton-johansson
Copy link

I'm getting this in 0.67.0 and I'm using @nivo/core and @nivo/line. My package-lock.json states that recompose is used by @nivo/core.

@Meaheesha
Copy link

I am also getting in 0.67.0. I am getting this as security vulnerability for @nvivo/bar.

@plouc
Copy link
Owner

plouc commented Dec 18, 2020

Please note that until this list is fully completed, you're going to get those warnings, we're aware and working on it!

@wyze took care of the @nivo/sunburst package.

@plouc plouc assigned wyze and plouc Dec 22, 2020
@falsepopsky
Copy link

falsepopsky commented Jan 4, 2021

Just to let you know guys, I got this warning in @nivo/heatmap "^0.67.0", I'm using responsive canvas.
ty in advance.

@GregdTd
Copy link

GregdTd commented Jan 5, 2021

Any news on @nivo/bar ? I'm still having the warning in "@nivo/bar": "^0.66.0"

@JacobMGEvans
Copy link

JacobMGEvans commented Feb 3, 2021

Just bumping, looks like this is happening in core which is still showing in this issue as not refactored yet.

console.warn
    Warning: React.createFactory() is deprecated and will be removed in a future major release. Consider using JSX or use React.createElement() directly instead.
    ...
    ...
   at enhance (node_modules/@nivo/voronoi/src/enhance.js:19:5)

sfount added a commit to alphagov/pay-toolbox that referenced this issue Feb 7, 2021
`node-fetch` has been fixed at <= 2.6.0 in the `@nivo` libraries, there
is a tracked removal of these deprecated methods
plouc/nivo#884.

This has been ongoing for roughly a year and looks like it won't be
patched in all the dependent modules (nivo/line) for a while.

The only parts of this app that use nivo/ react is the live payments
dashboard, the proposal is to split this out into its own component
repository as these kind of issues shouldn't reuqire maintenance for Pay
backend devs.

In the meantime none of the nivo components for the live payments
dashboard use asynchronous loading, as this is never used it is safe to
patch out the library failing npm audit (node fetch) until these
dependencies are moved over to a separate component repository and
handled and tracked over there.
sfount added a commit to alphagov/pay-toolbox that referenced this issue Feb 8, 2021
`node-fetch` has been fixed at <= 2.6.0 in the `@nivo` libraries, there
is a tracked removal of these deprecated methods
plouc/nivo#884.

This has been ongoing for roughly a year and looks like it won't be
patched in all the dependent modules (nivo/line) for a while.

The only parts of this app that use nivo/ react is the live payments
dashboard, the proposal is to split this out into its own component
repository as these kind of issues shouldn't reuqire maintenance for Pay
backend devs.

In the meantime none of the nivo components for the live payments
dashboard use asynchronous loading, as this is never used it is safe to
patch out the library failing npm audit (node fetch) until these
dependencies are moved over to a separate component repository and
handled and tracked over there.
sfount added a commit to alphagov/pay-toolbox that referenced this issue Feb 8, 2021
`node-fetch` has been fixed at <= 2.6.0 in the `@nivo` libraries, there
is a tracked removal of these deprecated methods
plouc/nivo#884.

This has been ongoing for roughly a year and looks like it won't be
patched in all the dependent modules (nivo/line) for a while.

The only parts of this app that use nivo/ react is the live payments
dashboard, the proposal is to split this out into its own component
repository as these kind of issues shouldn't reuqire maintenance for Pay
backend devs.

In the meantime none of the nivo components for the live payments
dashboard use asynchronous loading, as this is never used it is safe to
patch out the library failing npm audit (node fetch) until these
dependencies are moved over to a separate component repository and
handled and tracked over there.
@cfecherolle
Copy link

Hi, I'm getting this warning using only @nivo/core and @nivo/line but I don't see either of them in the list, are they planned for refactoring as well? Sorry if I missed something in this discussion but I would like to be sure.

@stephent
Copy link
Author

I saw that @plouc expanded the initial issue I reported, but like @cfecherolle I'm only using @nivo/core and @nivo/line also, so I believe they (or one of them) should be included in the list too?

@rap2hpoutre
Copy link

Maybe we could do a temporary fix, by using the fork of recompose that doesn't have security issues: https://github.com/shakacode/recompose.

It would be an intermediate step before doing something more difficult like completely remove the recompose package. Since it cost less time and less test, it could be faster for @plouc to accept PR before working on the full fix that removes recompose. What do you think?

Source: acdlite/recompose#817 (comment)

@rap2hpoutre
Copy link

Also: does @plouc still merge PR/maintain this project actively?

This was referenced Apr 21, 2021
@wyze wyze changed the title Warning: React.createFactory() is deprecated Tracking issue for removing recompose dependency Jun 9, 2021
@wyze wyze added the pinned label Jun 10, 2021
@MaffooBristol
Copy link

  • Had error in bar, circle-packing, line, radar and swarmplot versions 0.61.1
  • Upgraded to 0.70.1 and the above error went, but now got Jest errors regarding Cannot find module '@nivo/core' from 'nivo-radar.cjs.js'
  • Installed @nivo/[email protected]
  • Initial error is back 😓

@yuval9313
Copy link

Hey is this still a thing?
This lib is still maintained?

We also need support for react 18.0 by now, is that gonna be an issue?

@tkonopka
Copy link
Contributor

Of the open tasks, there is already a PR for waffle, so only core is outstanding. @plouc Would it be OK for me to have a look at that? It is a central component, so I understand if you'd rather leave it to someone with more nivo and js experience.

@plouc
Copy link
Owner

plouc commented May 3, 2023

Removal from @nivo/waffle in progress (#2320).

@plouc plouc mentioned this issue May 3, 2023
@plouc
Copy link
Owner

plouc commented May 7, 2023

Removed from @nivo/waffle (#2320).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.