Skip to content

Add missing htmlAttributes and bodyAttributes to develop-static-entry.js (Gatsby v1) #8596

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 3, 2018
Merged

Add missing htmlAttributes and bodyAttributes to develop-static-entry.js (Gatsby v1) #8596

merged 1 commit into from
Oct 3, 2018

Conversation

@taktran
Copy link
Contributor

@taktran taktran commented Sep 27, 2018

Fix for #8569 for Gatsby v1

Same fix as #8594, but I had issues running it with gatsby-dev-cli with gatsby v1 to check

@taktran taktran requested review from a team September 27, 2018 20:55
@taktran taktran changed the base branch from master to v1 September 27, 2018 20:55
@taktran taktran changed the title Topics/v1 fix html body attributes Add missing htmlAttributes and bodyAttributes to develop-static-entry.js (Gatsby v1) Sep 27, 2018
@pieh
Copy link
Contributor

pieh commented Sep 28, 2018

Can you try updating gatsby-cli to gatsby-cli@2.4.2 (yesterday v1 compatiblity fix was published)

@shannonbux
Copy link
Contributor

shannonbux commented Oct 2, 2018

@taktran what kind of review would you like from the docs team?

@pieh
Copy link
Contributor

pieh commented Oct 2, 2018

@shannonbux I think it's github "mistake" - github automatically asked for docs review earlier because originally PR was against master and would contain docs changes, but it was changed later to v1 branch and review request didn't adjust after that

@taktran
Copy link
Contributor Author

taktran commented Oct 3, 2018

Ok, I've tested the fix works on gatsby-bug@v1-fix-html-body-attributes

To check:

  • Run gatsby@v1 locally, with yarn && yarn run watch
  • Check out gatsby-bug@v1-fix-html-body-attributes and run gatsby-dev (from gatsby-dev-cli)
    • Run yarn run develop, and see that at http://localhost:8000/, the <html> and <body> tags don't have any attributes
    • To see the fix, in the gatsby repo, add my fix with git remote add tak-fix git@github.com:taktran/gatsby.git and git pull tak-fix topics/v1-fix-html-body-attributes
      • Now rerun the yarn run develop command, and you will see that at http://localhost:8000/, <html> and <body> tags both have attributes

However, from the looks of this PR, the problem seems to be that circle ci has not been migrated to circle 2. I'm not sure if it's worth doing for the v1 branch of gatsby or not. I've run the tests locally on topics/v1-fix-html-body-attributes, and they pass:

Test Suites: 80 passed, 80 total
Tests:       2 skipped, 629 passed, 631 total
Snapshots:   4 added, 209 passed, 213 total
Time:        11.819s, estimated 12s
Ran all test suites.
✨  Done in 21.50s.

@taktran taktran force-pushed the topics/v1-fix-html-body-attributes branch from b07f8a5 to ac52cd2 Compare October 3, 2018 22:20
@pieh pieh merged commit 62713c3 into gatsbyjs:v1 Oct 3, 2018
@pieh
Copy link
Contributor

pieh commented Oct 5, 2018

Published gatsby@1.9.279

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@taktran @pieh @shannonbux