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

Add environment variable to control image inlining threshold #6060

Conversation

peterbe
Copy link

@peterbe peterbe commented Dec 18, 2018

Fixes #3437

I verified that this worked by editing the default application (packages/react-scripts/template/src/App.js) to reference a logo.png image that I made up. The file was 5,237 bytes and when I ran yarn run build the build/static/js/main.8204aa3b.chunk.js did contain the base64 string. Then, running IMAGE_INLINE_SIZE_LIMIT=5000 yarn run build this time the build/static/js/main.499ec856.chunk.js file was -5.27KB and the file build/static/media/logo.adb700a2.png existed.

@peterbe
Copy link
Author

peterbe commented Dec 18, 2018

Can someone guide me in what's wrong with those two failing Travis builds?

@peterbe
Copy link
Author

peterbe commented Jan 10, 2019

@Timer apart from the conflict, which I can fix, can you help me with the failing Travis test and a review?

@peterbe peterbe force-pushed the 3437-environment-variable-to-disable-inlining-images branch from c1d1606 to ca6f07e Compare January 10, 2019 15:00
@stale
Copy link

stale bot commented Feb 9, 2019

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Feb 9, 2019
@stale stale bot removed the stale label Feb 9, 2019
@stale
Copy link

stale bot commented Mar 11, 2019

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

peterbe and others added 3 commits April 1, 2019 14:58
…ble-inlining-images' into 3437-environment-variable-to-disable-inlining-images
…able-inlining-images

3437 environment variable to disable inlining images
@rohan-deshpande
Copy link

rohan-deshpande commented Apr 13, 2019

I need this functionality, but the other way round, ie., I want everything to be base64 encoded.

Would be good to see this get merged. Seems like the issues are arising due to a missing module?

Error: [BABEL] /tmp/tmp.74WMXrb0ri/test-app-typescript/src/index.tsx: Cannot find module '@babel/plugin-transform-react-jsx-source' (While processing: "/tmp/tmp.74WMXrb0ri/test-app-typescript/node_modules/babel-preset-react-app/index.js$1")

@Rogdham
Copy link

Rogdham commented Jun 13, 2019

The corresponding issue #3437 has got some traction in the meanwhile, with different people saying that it is quite an issue for them due to CSP blocking data:, and that they would like this PR to be merged.

@iansu I see you assigned yourself on this PR a while ago. Could you please tell us how we should proceed for this PR to be merged? What are the required steps? 🙏

@ianschmitz ianschmitz changed the title environment variable to disable inlining images Add environment variable to control image inlining threshold Jun 14, 2019
@ianschmitz ianschmitz added this to the 3.0.2 milestone Jun 18, 2019
@ianschmitz ianschmitz merged commit 9d70c7a into facebook:master Jun 18, 2019
@ianschmitz
Copy link
Contributor

Thanks @peterbe!

@lock lock bot locked and limited conversation to collaborators Jun 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable inline images for images that are less than 10,000 bytes
8 participants