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

Support React 17 #1062

Closed
mquandalle opened this issue Nov 18, 2020 · 49 comments
Closed

Support React 17 #1062

mquandalle opened this issue Nov 18, 2020 · 49 comments

Comments

@mquandalle
Copy link

mquandalle commented Nov 18, 2020

NPM v7 throws the following error when using react-pdf with the latest version of React

npm ERR! node_modules/react
npm ERR!   react@"^17.0.0" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^16.0.0" from @react-pdf/renderer@1.6.12
npm ERR! node_modules/@react-pdf/renderer
npm ERR!   @react-pdf/renderer@"^1.6.10" from the root project

I'm not sure if there are any code migration required or if only the package.json needs to me modified.

@acomanescu
Copy link

Any update on this?

@xavierraffin
Copy link

Interested too

@HaykMan95
Copy link

it might help you npm install @react-pdf/renderer --legacy-peer-deps

@acomanescu
Copy link

it might help you npm install @react-pdf/renderer --legacy-peer-deps

This is a quick fix that solves the situation temporary, but if you remove the lock file, you have to use it again. It is not a clean solution...

@princefishthrower
Copy link

Made a pull request to bump react version here: #1157
You can meanwhile just fork this and issue npm update react, it works fine and then all tests pass, not sure where any maintainers are on this one, the original React 17 version issue was raised last October...

@diegomura
Copy link
Owner

diegomura commented Mar 29, 2021

Hi! Sorry for the delay!
What would happen to projects using React 16?
Don't have much time to test right now but I'd love to merge this asap

@acomanescu
Copy link

@diegomura Maybe you can add both, with version range, until you test it.

@diegomura
Copy link
Owner

diegomura commented Apr 6, 2021

So I tried upgrading react's peer dependency from ^16.8.6 to >16.8.6 that matches new React 17 version, but NPM v7 stills throws peer dependency errors due to react-reconciler@0.23.0 which has react@16 as peer dependency as well...

The newest react-reconciler@0.26.2 has react@^17.0.0 as peer dependency, but if I'm not mistaken will break react-pdf to anyone using react@16, which is still a lot of people (EDIT: I tried this, and it does break things to people using react@16)

It seems that the cause of this is NPMv7 now installs peer dependencies and breaks things (this is apparently breaking the react library ecosystem, not just this package). I'm open to suggestions here but I do not see how I can keep supporting all react versions and making NPMv7 to work, unless you use the --legacy-peer-dep flag as recommended above.

For those blocked by this, it is not that you cannot use the lib with React17 and NPM7, it jus throws this error when installing

@diegomura
Copy link
Owner

diegomura commented Apr 6, 2021

Just published 1.6.16 with the change suggested by @princefishthrower above, but I think it's not enough to make this whole peerDependency check to work. Please try it and let me know!

@maxdev
Copy link

maxdev commented Apr 7, 2021

@diegomura I tried on 2.0.0 and 1.6.16, but unfotunately it doesn't work without --legacy-peer-dep flag

@diegomura
Copy link
Owner

Yes. As explained above, I couldn't find the way of keep supporting react 16 and 17. This whole peer dependency check in nvm@7 breaks this library. The issue is not just react-pdf peer dependency but specially react-reconciler peer dependency as well. If I define both react-reconciler versions on our side, npm still does not infer the correct version it has to use. I'm open to suggestions here

@maxdev
Copy link

maxdev commented Apr 8, 2021

Hi @diegomura

I see the problem. Maybe solution with aliases could help in this case? https://dev.to/joshx/test-your-npm-package-against-multiple-versions-of-its-peer-dependency-34j4

@diegomura
Copy link
Owner

diegomura commented Apr 8, 2021

Interesting.. But I don't think npm aliases can solve this. I will still have to list a react-reconciler version with a peer dependency of react@16, so the check will still fail, right?. I thought that by defining "react-reconciler: ^0.23.0 || ^0.26.0" npm would automatically choose the version who's peer dependencies best adjusts to your dependencies (0.23.0 having react@16 and 0.26.0 react@17 respectively). But no, it always end up installing 0.26.0, breaking all those projects that uses react@16.

React official renderers builds, such as react-dom or react-native, are shipped with the react-reconciler code embedded in the build (I mean, not as an external dependency). This is something I guess I can do for this project as well, although it just feels wrong to publish this package with a third-party dependency built inside of it

@maxdev
Copy link

maxdev commented Apr 8, 2021

Hi @diegomura

React official renderers builds, such as react-dom or react-native, are shipped with the react-reconciler code embedded in the build (I mean, not as an external dependency). This is something I guess I can do for this project as well, although it just feels wrong to publish this package with a third-party dependency built inside of it

I think it's a good point.

Interesting.. But I don't think npm aliases can solve this. I will still have to list a react-reconciler version with a peer dependency of react@16, so the check will still fail, right?. I thought that by defining "react-reconciler: ^0.23.0 || ^0.26.0" npm would automatically choose the version who's peer dependencies best adjusts to your dependencies (0.23.0 having react@16 and 0.26.0 react@17 respectively). But no, it always end up installing 0.26.0, breaking all those projects that uses react@16.

Maybe I'm completely wrong, but I have an assumption why npm always installs latest minor version, probably because ^0.23.0 is equivalent to ^0.26.0?
What if to try something like "react-reconciler: 0.23.0 || ^0.26.0"

@diegomura
Copy link
Owner

Thanks @maxdev ! That might be right. Will try today and report back here

@diegomura
Copy link
Owner

diegomura commented Apr 8, 2021

Unfortunately this still happens :(

Screen Shot 2021-04-08 at 3 04 20 PM

npm keeps insisting on installing react-reconciler@0.26.0 even if react@16 is present

@KarlBaumann
Copy link

I can confirm that it is still broken

@diegomura
Copy link
Owner

Thanks @KarlBaumann. It is :( but as explained above I'm on a dead end for both supporting react 16 and 17 with the new npm peer dependency install

@KarlBaumann
Copy link

@diegomura
I would say you don't have to support outdated libraries in latest versions. If people want to use outdated software, they can use older releases.

@diegomura
Copy link
Owner

While I generally agree, with React for me it's more complex. Since it's the main peer dependency of this project and one of those libraries that people usually do not update right away, just stopping support for React 16 would leave many people behind. It's not that people using React17 cannot use latest build either. For what I understood, the only issue is with the latest npm peer dependency check, right?

@KarlBaumann
Copy link

KarlBaumann commented Apr 18, 2021

for me it does not work also with the peer-dependency-check disabled. It is totally normal that major releases introduce breaking changes. Thats why I would expect that everyone who is already using the library, has something like this ^2.0.0 in their package.lock file, so your version 3 would not break their apps.

@mdodge-ecgrow
Copy link

Just curious, how long do you forsee supporting React 16?

@diegomura
Copy link
Owner

I don't have any fixed plan. I'm open to suggestions 😄 Would still love to see if there's some solution to this. @KarlBaumann releasing a 3.x it's not an option since that would be even more confusing

@grmatthews
Copy link

How difficult would it be to create a React 17 compatible version as a temp measure for those of us on React 17? i.e. leave 16 as the main supported version it's time for the whole thing to officially move to be based on React 17?

@benjamin-andersen
Copy link

1+. This library seems super useful and our team would like to use it in our production application to generate a PDF and download it. We use React 17 however and there aren't many other alternatives that provide a solution to this :(

I think making a separate version for React 17 and one for React 16 would be a good way to temporarily solve this issue as mentioned by @grmatthews.

@sm3sher
Copy link

sm3sher commented Aug 17, 2021

Waiting for React 17 support

@yourjhay
Copy link

yourjhay commented Sep 6, 2021

Waiting for React 17 Support, Planning to use this lib in our production env

@hobbitjeditransgender
Copy link

Any update here? @diegomura

@muhsalaa
Copy link

Waiting for React 17 Support

@TD-DO
Copy link

TD-DO commented Nov 4, 2021

Would also like React 17 support

@indignant
Copy link

I was curious how other packages are handling this. react-router is using 17 and react-redux uses both. For what it's worth, Wes Bos recommends the same thing as @mquandalle.

As of the end of October Node 16 (NPM 7) is the active LTS; I think this is going to be increasingly problematic as people start switching over and I think it should be a quick fix. Would be glad to submit a PR with maybe some hints on how I can test.

@vegai
Copy link

vegai commented Feb 21, 2022

PR above implements what mquandelle suggested on May 10, 2021. Seems to fix the problem in our end at least.

@dudusohne
Copy link

Still doesn't worked for me

@karlhorky
Copy link
Contributor

@diegomura with the React version 18 release just around the corner, what do you think about upgrading to React 17 (dropping support for React 16) and publishing a new major version of @react-pdf/renderer?

@karlhorky
Copy link
Contributor

One additional detail here is that upgrading to React 17 enables usage of the new JSX transform, which would allow for leaving out the import React from 'react' in every file! 🙌

@karlhorky
Copy link
Contributor

karlhorky commented Mar 25, 2022

Just ran into a problem trying to use @react-pdf/renderer with the new transform, actually.

The incompatibility with React 17 is causing an error like this:

Uncaught ReferenceError: React is not defined

This was caused by the "jsx": "react-jsx" in my TypeScript tsconfig.json, which specifies the new transform:

{
  "compilerOptions": {
    "jsx": "react-jsx"
  }
}

Workaround 1

To work around @react-pdf/renderer's incompatibility with the new JSX transform, I needed to:

  1. Change jsx to preserve (see below) to get @react-pdf/renderer to work
  2. Add the import React from 'react' to the file using @react-pdf/renderer
{
  "compilerOptions": {
-    "jsx": "react-jsx"
+    "jsx": "preserve"
  }
}

Workaround 2

The workaround described under Workaround 1 above caused weird build errors:

$ yarn build
✨  Done in 7.81s.

$ yarn start
node:internal/errors:464
    ErrorCaptureStackTrace(err);
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/home/runner/work/build/learningOutcomesPdf.js' imported from /home/runner/work/build/index.js
    at new NodeError (node:internal/errors:371:5)
    at finalizeResolution (node:internal/modules/esm/resolve:418:11)
    at moduleResolve (node:internal/modules/esm/resolve:981:10)
    at defaultResolve (node:internal/modules/esm/resolve:1078:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:530:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:251:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:79:40)
    at link (node:internal/modules/esm/module_job:78:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

We took a look inside the build folder, and indeed, the file had a .jsx extension!

$ ls build/
learningOutcomesPdf.jsx

This is because TypeScript builds .tsx files with the .jsx extension when "jsx": "preserve" is configured in the tsconfig.json: https://www.typescriptlang.org/docs/handbook/jsx.html#basic-usage

So we couldn't use the preserve configuration option.

Instead, we removed that "jsx": "preserve" from tsconfig.json and added the following to the top of our learningOutcomesPdf.tsx file:

/** @jsxImportSource react */

This almost worked! 🙌 And probably would work for many teams.

But because our package is a Node.js ESM package, we had further problems with Node.js not resolving react/jsx-runtime:

$ yarn start
node:internal/process/esm_loader:74
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/k/p/project/node_modules/react/jsx-runtime' imported from /Users/k/p/project/learningOutcomesPdf.js
Did you mean to import react/jsx-runtime.js?
    at new NodeError (node:internal/errors:371:5)
    at finalizeResolution (node:internal/modules/esm/resolve:416:11)
    at moduleResolve (node:internal/modules/esm/resolve:932:10)
    at Loader.defaultResolve [as _resolve] (node:internal/modules/esm/resolve:1044:11)
    at Loader.resolve (node:internal/modules/esm/loader:89:40)
    at Loader.getModuleJob (node:internal/modules/esm/loader:242:28)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:40)
    at link (node:internal/modules/esm/module_job:75:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

For this, we needed to patch React 17 with the "exports" field for package.json (which is also coming in React 18):

diff --git a/node_modules/react/package.json b/node_modules/react/package.json
index 936478a..65e2461 100644
--- a/node_modules/react/package.json
+++ b/node_modules/react/package.json
@@ -19,6 +19,11 @@
     "jsx-dev-runtime.js"
   ],
   "main": "index.js",
+  "exports": {
+    ".": "./index.js",
+    "./jsx-dev-runtime": "./jsx-dev-runtime.js",
+    "./jsx-runtime": "./jsx-runtime.js"
+  },
   "repository": {
     "type": "git",
     "url": "https://github.com/facebook/react.git",

Ref: facebook/react#20235 (comment)

@karlhorky
Copy link
Contributor

cc @jeetiss, since it seems that you are also an active maintainer here

@pascalporedda
Copy link

pascalporedda commented Apr 6, 2022

Gonna +1 the official support of React 18. So far we had no problem running this with React 17, by simply using the legacy peer deps, haven't tried v18 though.

@karlhorky
Copy link
Contributor

@react-pdf/renderer is working also with React 18 for us in production - after our workarounds for React 17

@karlhorky
Copy link
Contributor

karlhorky commented Apr 7, 2022

Hm, but it fails with @types/react v18, which was just published (because of the breaking changes):

Run yarn tsc
yarn run v1.22.18
$ /home/runner/work/project/node_modules/.bin/tsc
Error: project/generatePdf.tsx(212,6): error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: DocumentProps | Readonly<DocumentProps>): Document', gave the following error.
    Type '{ children: Element[]; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Document> & Readonly<DocumentProps>'.
  Overload 2 of 2, '(props: DocumentProps, context: any): Document', gave the following error.
    Type '{ children: Element[]; }' has no properties in common with type 'IntrinsicAttributes & IntrinsicClassAttributes<Document> & Readonly<DocumentProps>'.
Error: project/generatePdf.tsx(213,8): error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: PageProps | Readonly<PageProps>): Page', gave the following error.
    Type '{ children: Element; size: "A4"; style: { fontFamily: string; fontSize: number; lineHeight: number; paddingTop: number; paddingBottom: number; paddingHorizontal: number; }; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
  Overload 2 of 2, '(props: PageProps, context: any): Page', gave the following error.
    Type '{ children: Element; size: "A4"; style: { fontFamily: string; fontSize: number; lineHeight: number; paddingTop: number; paddingBottom: number; paddingHorizontal: number; }; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
Error: packages/learn.upleveled.io-server/routes/studentAssessments/learningOutcomesPdf.tsx(259,12): error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: SVGProps | Readonly<SVGProps>): Svg', gave the following error.
    Type '{ children: Element[]; style: { marginTop: number; marginBottom: number; }; width: string; height: string; viewBox: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Svg> & Readonly<SVGProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Svg> & Readonly<SVGProps>'.
  Overload 2 of 2, '(props: SVGProps, context: any): Svg', gave the following error.
    Type '{ children: Element[]; style: { marginTop: number; marginBottom: number; }; width: string; height: string; viewBox: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Svg> & Readonly<SVGProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Svg> & Readonly<SVGProps>'.
Error: project/generatePdf.tsx(266,16): error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: LinearGradientProps | Readonly<LinearGradientProps>): LinearGradient', gave the following error.
    Type '{ children: Element[]; id: string; x1: string; x2: string; y1: string; y2: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<LinearGradient> & Readonly<LinearGradientProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<LinearGradient> & Readonly<LinearGradientProps>'.
  Overload 2 of 2, '(props: LinearGradientProps, context: any): LinearGradient', gave the following error.
    Type '{ children: Element[]; id: string; x1: string; x2: string; y1: string; y2: string; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<LinearGradient> & Readonly<LinearGradientProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<LinearGradient> & Readonly<LinearGradientProps>'.
Error: project/generatePdf.tsx(285,8): error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: PageProps | Readonly<PageProps>): Page', gave the following error.
    Type '{ children: Element; size: "A4"; style: { fontFamily: string; fontSize: number; lineHeight: number; paddingTop: number; paddingBottom: number; paddingHorizontal: number; }; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
  Overload 2 of 2, '(props: PageProps, context: any): Page', gave the following error.
    Type '{ children: Element; size: "A4"; style: { fontFamily: string; fontSize: number; lineHeight: number; paddingTop: number; paddingBottom: number; paddingHorizontal: number; }; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
      Property 'children' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Page> & Readonly<PageProps>'.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 2.

@karlhorky
Copy link
Contributor

karlhorky commented Apr 24, 2022

Hm, but it fails with @types/react v18, which was just published (because of the breaking changes)

Found some time to take a look at this, and opened a PR to fix the types in the @react-pdf/renderer package: #1798

However, this does not fix the other packages in the monorepo. And this may cause breakage for @types/react versions lower than 18.

cc @diegomura @jeetiss

@mabreu0
Copy link

mabreu0 commented May 5, 2022

In the meantime, while i get more insight about this.. Watchers please drop any light if any!

(This is using --legacy-peer-deps alternative with react-scripts 5.0.1 / react 18)

Creating an optimized production build...
Failed to compile.

Module not found: Error: Can't resolve 'stream' in '...../node_modules/@react-pdf/pdfkit/lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
- install 'stream-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "stream": false }

@MirKml
Copy link

MirKml commented Jun 9, 2022

I think that possible solution is use react-reconciler as peerDep for @react-pdf/renderer itself.
Renderer itself than can be backward compatible for react16 - as @diegomura wants. It's up to user which version of reconciler will be used.
Reconciler itself has peerDep for correct react major version.
So then end user has minimal space for error. Maybe npm 7+ with new peerDep auto solving process can install correct version of reconciler. If not, there will be information in install section from package autor :-)

Nowadays, latest major @react-pdf/renderer declares compatibility with react 17 via peerDep, but has runtime dependency for old react-reconciler - 0.23, which isn't compatible with react 17 but it works together. Magic from user point of view :-)
When user tries (doesn't matter how :-)) 'correct' react-reconciler 0.26 - version for react 17 - installation is without any warnings but renderer fails in runtime. I think it's weird and has to be fixed.

@ops1ops
Copy link

ops1ops commented Sep 15, 2022

Any news? Will the fate of the enzyme overtake this library? Its been 2 years but you still dont support React 17...

@andfaulkner
Copy link

andfaulkner commented Dec 23, 2022

Still seeking React 17 support at least (if not React 18).

That'd be much appreciated :)

I'm considering forking it for now and implementing the above changes since I'm pretty reliant on this library, but that's obviously not an ideal solution. I can try pushing a PR back into the project if I get it working...but I'd have to get a go-ahead for something that might harm React 16 support (which IMHO should just be dropped at this point, since it's now 2 major versions behind and has dropping market share).

@jeetiss
Copy link
Collaborator

jeetiss commented Jan 28, 2023

should be fixed in #2140

@jeetiss jeetiss closed this as completed Jan 28, 2023
jeetiss added a commit that referenced this issue Jan 28, 2023
* Fix types for React v18

Ref: #1062 (comment)
Ref: https://github.com/eps1lon/types-react-codemod#implicit-children
Ref: DefinitelyTyped/DefinitelyTyped#46691

* add changeset

---------

Co-authored-by: Dmitry Ivakhnenko <jeetiss@yandex.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests