Skip to content

Conversation

@DuncanMacWeb
Copy link
Contributor

Once finished along with react-pdf/pdfkit#7, this would resolve #507.

expect(dummyRoot.instance.image.mock.calls[0][0]).toBe(image.image.data);
});

/* test('Should render a buffer gif image', async () => {
Copy link
Contributor Author

@DuncanMacWeb DuncanMacWeb Mar 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test currently makes the test runner hang, which is why I’ve commented it out. Haven’t worked out why it’s hanging yet 🤔

@DuncanMacWeb
Copy link
Contributor Author

DuncanMacWeb commented Mar 14, 2019

It may not be necessary to use omggif at all if we decide not to decode the GIF image data as proposed in react-pdf/pdfkit#7

@DuncanMacWeb DuncanMacWeb force-pushed the support-gif-images branch 4 times, most recently from 1b097a4 to 4d90ed2 Compare March 31, 2019 16:04
@DuncanMacWeb DuncanMacWeb marked this pull request as ready for review March 31, 2019 17:36
@DuncanMacWeb DuncanMacWeb marked this pull request as ready for review March 31, 2019 17:36
if (!BROWSER) {
const pngBuffer = await sharp(data).toBuffer();
return new PNG(pngBuffer);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an issue with the way Rollup bundles this — because it doesn’t do dead-code elimination exactly, the browser builds still import the server-side sharp module, so — even though the BROWSER global is set to true — when it’s consuming the build Webpack processes the import sharp from 'sharp' statement and gives this error:

WARNING: Critical dependency: the request of a dependency is an expression (expand for details)  
WARNING in ./node_modules/sharp/lib/libvips.js 51:22-69
Critical dependency: the request of a dependency is an expression
@ ./node_modules/sharp/lib/constructor.js
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss

WARNING in ./node_modules/sharp/lib/libvips.js 52:23-70
Critical dependency: the request of a dependency is an expression
@ ./node_modules/sharp/lib/constructor.js
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss
ERROR: Module not found: Error: Can't resolve 'child_process' (expand for details)  
ERROR in ./node_modules/detect-libc/lib/detect-libc.js
Module not found: Error: Can't resolve 'child_process' in './node_modules/detect-libc/lib'
@ ./node_modules/detect-libc/lib/detect-libc.js 4:16-40
@ ./node_modules/sharp/lib/platform.js
@ ./node_modules/sharp/lib/libvips.js
@ ./node_modules/sharp/lib/constructor.js
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss

ERROR in ./node_modules/sharp/lib/libvips.js
Module not found: Error: Can't resolve 'child_process' in './node_modules/sharp/lib'
@ ./node_modules/sharp/lib/libvips.js 6:18-42
@ ./node_modules/sharp/lib/constructor.js
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss

ERROR in ./node_modules/bindings/bindings.js
Module not found: Error: Can't resolve 'fs' in './node_modules/bindings'
@ ./node_modules/bindings/bindings.js 5:9-22
@ ./node_modules/sharp/lib/constructor.js
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss

ERROR in ./node_modules/detect-libc/lib/detect-libc.js
Module not found: Error: Can't resolve 'fs' in './node_modules/detect-libc/lib'
@ ./node_modules/detect-libc/lib/detect-libc.js 5:18-31
@ ./node_modules/sharp/lib/platform.js
@ ./node_modules/sharp/lib/libvips.js
@ ./node_modules/sharp/lib/constructor.js
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss

ERROR in ./node_modules/sharp/lib/libvips.js
Module not found: Error: Can't resolve 'fs' in './node_modules/sharp/lib'
@ ./node_modules/sharp/lib/libvips.js 3:11-24
@ ./node_modules/sharp/lib/constructor.js
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss

ERROR in ./node_modules/sharp/build/Release/sharp.node 1:0
Module parse failed: Unexpected character '�' (1:0)
You may need an appropriate loader to handle this file type.
(Source code omitted for this binary file)
@ ./node_modules/sharp/lib/input.js 5:14-52
@ ./node_modules/sharp/lib/index.js
@ ./node_modules/@react-pdf/renderer/dist/react-pdf.browser.es.js
@ ./src/client/components/xocs/jsx-components.pdf.js
@ ./src/client/clientRender.pdf.js
@ ./src/client/index.pdf.js
@ multi (webpack)-dev-server/client?http://localhost:8082 (webpack)/hot/dev-server.js ./src/client/index.pdf ./src/client/style.scss
ℹ 「wdm」: Failed to compile.

Do you know of a way to shake the non-BROWSER code completely out of Rollup’s browser builds? :)

@DuncanMacWeb
Copy link
Contributor Author

DuncanMacWeb commented Apr 1, 2019

@diegomura I didn’t work out how to push the GIFs’ LZW-compressed image data into the PDF partly due to build issues as discussed in react-pdf/pdfkit#7, so in the end I’m converting the images to PNG (server-side) and JPG (client-side), as it seemed the quickest way of getting the GIF images in. Definitely room to improve on this but in principle, it does the job for now.

Would you be able to review this please — it needs testing against GIFs and client- and server-side. There are also a couple of specific issues around tests and bundling which I’ve commented on above. Thanks!

@diegomura
Copy link
Owner

I think this can be easily ported to 2.0. The only think that worries me is the several dependencies this PR adds. How much bundle size will add to the lib?

@diegomura diegomura closed this Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image rendering — GIF support

2 participants