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): Show meaningful error when directory names are too long #21518

Merged
merged 29 commits into from Mar 11, 2020

Conversation

jlkiri
Copy link
Contributor

@jlkiri jlkiri commented Feb 17, 2020

Description

Prevent build from crashing when page path lengths (directory name lengths) exceed OS limit (255 bytes). Truncate it to safe length and hash what remains for uniqueness. URLs are kept original for SEO reasons. Mapping from original URL to real disk path is done inside express middleware.

Details in #20699

WIP because I am not sure whether adding one more middleware is a good idea. Not sure whether I forgot to fix some other logic.

I am going to fix smaller details once this approach is approved.

Documentation

Maybe relevant: https://www.gatsbyjs.org/docs/actions/#createNode

Related Issues

#20699
#4125

@jlkiri jlkiri requested a review from a team as a code owner February 17, 2020 09:44
@jlkiri
Copy link
Contributor Author

jlkiri commented Feb 17, 2020

Possibly also related to #13631

@jlkiri
Copy link
Contributor Author

jlkiri commented Feb 18, 2020

@pieh I decided not to touch serve and went with the safer alternative. A user can opt into truncation in gatsby-config.js without further control over paths but still better than manual in cases where a user does not care about what a path looks like. By default truncateLongPaths is false and creating very long paths leads to a detailed error.

I also added some tests.

What do you think?

@jlkiri jlkiri requested a review from pieh February 18, 2020 10:13
@jlkiri jlkiri changed the title WIP: Prevent build from crashing when directory names are too long Prevent build from crashing when directory names are too long Feb 18, 2020
@jlkiri jlkiri self-assigned this Feb 22, 2020
@jlkiri
Copy link
Contributor Author

jlkiri commented Feb 22, 2020

Is there anything else to fix/take care of?

packages/gatsby/src/redux/actions/public.js Outdated Show resolved Hide resolved
@jlkiri jlkiri requested review from a team as code owners February 23, 2020 17:43
@jlkiri jlkiri requested a review from pieh February 24, 2020 09:11
@jlkiri jlkiri added the status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response label Mar 2, 2020
@pieh pieh changed the title Prevent build from crashing when directory names are too long fix(gatsby): Show meaningful error when directory names are too long Mar 11, 2020
@pieh pieh self-assigned this Mar 11, 2020
Copy link
Contributor

@pieh pieh 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 pieh merged commit 4404af1 into gatsbyjs:master Mar 11, 2020
@pieh
Copy link
Contributor

pieh commented Mar 12, 2020

Published gatsby@2.19.40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants