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 early bail when testing if string field should link to File node #6504

Merged
merged 5 commits into from Jul 18, 2018

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Jul 17, 2018

mime@^1.0.0 used to return 'application/octet-stream' for unkown filenames - this was changed in mime@^2.0.0 and now it returns null ( ref: broofa/mime#139 )

When we upgraded mime and didn't adjust this caused every string field to go through quite expensive File nodes lookup - causing very long schema generation in some cases.

Closes #6467

@gatsbybot
Copy link
Collaborator

gatsbybot commented Jul 17, 2018

Deploy preview for using-drupal ready!

Built with commit 77b340a

https://deploy-preview-6504--using-drupal.netlify.com

@gatsbybot
Copy link
Collaborator

gatsbybot commented Jul 17, 2018

Deploy preview for gatsbygram ready!

Built with commit 77b340a

https://deploy-preview-6504--gatsbygram.netlify.com

@KyleAMathews
Copy link
Contributor

@pieh
Copy link
Contributor Author

pieh commented Jul 18, 2018

Yeah, I checked that before and just didn't update it because it would work:

const mediaType = mime.getType(slashedFile.ext)
internal = {
contentDigest,
type: `File`,
mediaType: mediaType ? mediaType : `application/octet-stream`,
description: `File "${path.relative(process.cwd(), slashed)}"`,
}

this will actually keep v1 behaviour of setting application/octet-stream for File nodes if mime.getType will report null, but probably noone will test against either null or application/octet-stream

But I guess if it doesn't matter let's not diverge from current mime, so I'll just update it too

@KyleAMathews
Copy link
Contributor

Deploy preview for using-postcss-sass failed.

Built with commit 16e79e7

https://app.netlify.com/sites/using-postcss-sass/deploys/5b4f136f82d3f10a7424957a

@KyleAMathews
Copy link
Contributor

Deploy preview for image-processing failed.

Built with commit 16e79e7

https://app.netlify.com/sites/image-processing/deploys/5b4f137082d3f10a7424957e

@KyleAMathews
Copy link
Contributor

KyleAMathews commented Jul 18, 2018

Deploy preview for using-postcss-sass failed.

Built with commit 77b340a

https://app.netlify.com/sites/using-postcss-sass/deploys/5b4f2108c6aed65949b90ee0

@KyleAMathews
Copy link
Contributor

Deploy preview for image-processing failed.

Built with commit d9b0aff

https://app.netlify.com/sites/image-processing/deploys/5b4f13a582d3f116bf249549

Copy link
Contributor

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

👍 thanks @pieh

@m-allanson m-allanson merged commit a8124b0 into gatsbyjs:master Jul 18, 2018
mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
…atsbyjs#6504)

* fix early bail when for file inferring

* remove any `application/octet-stream` handling to be in line with mime@2 behaviour

* this can be shorter, doh!
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.

None yet

4 participants