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

fix(gatsby-source-filesystem): fix broken stream with gzipped files #28913

Merged
merged 5 commits into from
Jan 8, 2021

Conversation

wardpeet
Copy link
Contributor

@wardpeet wardpeet commented Jan 7, 2021

Description

When creating #28547 we didn't think of gzip, brotli compression when doing remote file downloads. SVGs for example have this issue. Costco reported this issue. I did a fix and created some integration tests to see that it actually works.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jan 7, 2021
@wardpeet wardpeet added status: needs core review Currently awaiting review from Core team member topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jan 7, 2021
Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

I love those integration tests! And the check at

responseStream.on(`downloadProgress`, progress => {
      if (progress.transferred === progress.total || progress.total === null)

is a lot less brittle. Thank you so much for this fix!

@sidharthachatterjee sidharthachatterjee merged commit a8b516f into gatsbyjs:master Jan 8, 2021
@sidharthachatterjee sidharthachatterjee removed the status: needs core review Currently awaiting review from Core team member label Jan 8, 2021
@wardpeet wardpeet deleted the fix/source-encoding branch January 8, 2021 14:02
yarn.lock Show resolved Hide resolved
@vladar
Copy link
Contributor

vladar commented Jan 16, 2021

Published in [email protected]

@ertrzyiks
Copy link

Is this fix available in @3.0.0 ? I had some issues with remote images and downgrading to 2.x helped.

@wardpeet
Copy link
Contributor Author

@ertrzyiks it should be, can you open an issue and a reproduction? 🙏

@ertrzyiks
Copy link

@wardpeet thanks for the quick answer, I will try to reproduce it in a simple setup

@rroslaniec
Copy link

@ertrzyiks it is related I think with my comment: #28983 (comment)

@ahmadkhalaf1
Copy link

ahmadkhalaf1 commented Dec 9, 2021

Published in [email protected]

Hello @vladar

seems like this fix was not merged into the next releases
trying [email protected] works tottaly fine

however on the same code and image trying it with version 3.0.0 or 3.14.0 , or even version 4 it shows the error .
is it possible to check this and merge it to the latest release for gatsby 3 please?

Thanks

@ahmadkhalaf1
Copy link

@wardpeet
can you please check if the fix that was published into [email protected] is released to the latest version ?
it does not work with Gatsby 3 or Gatsby 4
only when i set the version to 2.9.1 it work fine.
please answer me if u can
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants