diegomura / react-pdf

πŸ“„ Create PDF files using React
https://react-pdf.org
MIT License
14.28k stars 1.12k forks source link

Support React 17 #1062

Closed mquandalle closed 1 year ago

mquandalle commented 3 years ago

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.

arobert93 commented 3 years ago

Any update on this?

xavierraffin commented 3 years ago

Interested too

HaykMan95 commented 3 years ago

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

arobert93 commented 3 years ago

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 commented 3 years ago

Made a pull request to bump react version here: https://github.com/diegomura/react-pdf/pull/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 commented 3 years ago

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

arobert93 commented 3 years ago

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

diegomura commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

diegomura commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

diegomura commented 3 years ago

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 commented 3 years ago

I can confirm that it is still broken

diegomura commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

diegomura commented 3 years ago

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

mquandalle commented 3 years ago

Is it possible to use a β€œor” condition ?

    "react": "^16.0.0 || ^17.0.0",
    "react-dom": "^16.0.0 || ^17.0.0",
IsaacTrevino commented 2 years ago

+1

grmatthews commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Waiting for React 17 support

yourjhay commented 2 years ago

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

bbzitopirulito commented 2 years ago

Any update here? @diegomura

muhsalaa commented 2 years ago

Waiting for React 17 Support

TD-DO commented 2 years ago

Would also like React 17 support

indignant commented 2 years ago

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 commented 2 years ago

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

dudusohne commented 2 years ago

Still doesn't worked for me

karlhorky commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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: https://github.com/facebook/react/issues/20235#issuecomment-728824662

karlhorky commented 2 years ago

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

pascalporedda commented 2 years ago

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 commented 2 years ago

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

karlhorky commented 2 years ago

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 commented 2 years ago

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: https://github.com/diegomura/react-pdf/pull/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 commented 2 years ago

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:

MirKml commented 1 year ago

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 commented 1 year ago

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

andfaulkner commented 1 year ago

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 commented 1 year ago

should be fixed in #2140