Merged
Conversation
heyimalex
requested review from
Timer,
amyrlam,
bugzpodder,
gaearon,
ianschmitz,
iansu,
mrmckeb and
petetnt
as code owners
iansu
approved these changes
Jul 16, 2019
petetnt
approved these changes
Jul 18, 2019
Contributor
|
Let's see if we can get CI passing and then merge this. |
This closes facebook#7359 by making IMAGE_INLINE_SIZE_LIMIT=0 actually disable image inlining. We use url-loader's `limit` option to set the line at which images will be inlined, and we basically expose this option to our end users via the IMAGE_INLINE_SIZE_LIMIT environment variable. Most people using it want to disable inlining images all together (the major motivator for adding it at all was I think for strict CSP), and our documentation said setting the value to zero would do that. However, it did the opposite: url-loader treated zero as "no limit". The update to 2.x fixes this; zero means nothing gets inlined. The other changes from the version bump shouldn't affect us: - minimum node version is 8.9.x, we already require 8.10.x - the limit was "greater than", is now "greater than or equal to"
heyimalex
force-pushed
the
update_url_loader
branch
from
f5495fa to
58fad40
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This closes #7359 by making IMAGE_INLINE_SIZE_LIMIT=0 actually disable image inlining.
We use url-loader's
limitoption to set the line at which images will be inlined, and we basically expose this option to our end users via the IMAGE_INLINE_SIZE_LIMIT environment variable. Most people using it want to disable inlining images all together (the major motivator for adding it at all was I think for strict CSP), and our documentation said setting the value to zero would do that. However, it did the opposite: url-loader treated zero as "no limit".The update to 2.x fixes this; zero means nothing gets inlined. The other changes from the version bump shouldn't affect us:
Validated this change manually, checking whether the image was inlined at different vales of IMAGE_INLINE_SIZE_LIMIT, everything worked like a charm.