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

Draft: Issue 524/upgrade gatsby #562

Draft
wants to merge 18 commits into
base: develop
Choose a base branch
from
Draft

Conversation

naomatheus
Copy link
Collaborator

Responds to #524

@naomatheus
Copy link
Collaborator Author

naomatheus commented Jul 3, 2023

During local build the build is progressing. My last commit allows you to view a more verbose output of the nodes as they're being processed.
I'm observing the build getting stuck/slowing down a lot around the image url objects such as this:

{
  shortname: 'desert',
  category: 'region',
  image: 'https://earthobservatory.nasa.gov/blogs/fromthefield/wp-content/uploads/sites/2/2018/01/desert-plant-and-lidar.jpg',
  nasaImgAlt: 'The lidar on its tripod. (Photography courtesy NASA/GSFC)',
  id: '5d6b550e-b897-5075-97a7-3f0914051cac',
  children: [],
  parent: 'fc788723-1e74-55fb-b3a5-a4f40c0f447a',
  internal: {
    contentDigest: '43f23553f8e212d667b946ff798873e5',
    type: 'NasaImagesJson',
    owner: 'gatsby-transformer-json',
    counter: 10477
  }
}

For some reason these requests tend to timeout or take around 500s.
I'll try moving these images to a local directory and see if I can point this portion of the build toward that.

@naomatheus
Copy link
Collaborator Author

Yea, actually this build progress with the dependency updates. The NASA image from CDN are taking forever to build, I'm sure there's some reason for that, and it may be its own issue.

@Tammo-Feldmann
How extensive do you think the creating of Slices needs to be?
From what I see in the docs, it might make sense to move any "section component" in the src/components/layout directory to a Slice. Any thoughts?

@Tammo-Feldmann
Copy link
Contributor

Tammo-Feldmann commented Jul 3, 2023

@naomatheus, I'm not sure if you noticed this but I already have a PR open for the Gatsby update here. I'm sorry that I didn't communicate more clearly that I had already put some work into this. Will try to make that more obvious going forward.
As far as the Slicing goes, I honestly don't know. I'd like to try out the slice API and see if it can really make a difference. Seems like starting with header, hero, footer etc. would be the most intuitive place to start. If those really make a difference we could look for other places where we reuse components. I'm not sure about templates (like the ones that we use for campaigns for example).

@naomatheus
Copy link
Collaborator Author

Ah yes, @Tammo-Feldmann I did remember seeing that PR. I thought it'd be better to start a fresh branch, even if there was some slight duplication of work. I was able to work through updating the Reach dependencies and some other gatsby components that aren't supported in v5 (because they're built in). I'll make sure to merge integrate them somehow.

I am able to build this with Gatsby v5!

Yea, I saw your comment about styled components though.
Not sure if there's a workaround for that with the project. Are there any components that don't use styled components that we could use within a slice? From the chatter in that PR, it seems like while theoretically it could make sense to simply not use styled components in components that are part of slices that this doesn't work in practice.

@naomatheus
Copy link
Collaborator Author

Now it seems like an update to jest unit tests will be necessary to wrap up this upgrade.

@naomatheus
Copy link
Collaborator Author

updated notes

@naomatheus
Copy link
Collaborator Author

naomatheus commented Jul 17, 2023

After looking into the most up-to-date discussion on this issue that Gatsby 5 slice API has with styled-components, the last thing I have found is that they do not support this and don't have plans to offer specific support for react/styled-components. I see promising discussion on using emotion/styled and other emotion plugins, and that solution does seem to work. In most cases, excluding advanced uses, the API is highly similar, and conversion may require minimal refactoring labor.

https://emotion.sh/docs/styled

I have a list of existing files in the codebase that use styled-components. It's in 25 components, and these would need converting from any use of styled-components code to emotion/styled.

I'll try to plug a few of these components in a Gatsby slice and will check to see if the build succeeds and still shows the styling we want.

@Tammo-Feldmann
Copy link
Contributor

@naomatheus that's great news and really good work on your part. I'm happy to hear that you found emotion/styled as a very similar replacement. Will be curious to hear more about your findings. Sounds like this would be a great way forward for us if it works.

@naomatheus
Copy link
Collaborator Author

Yep. This seems promising. I'm going to convert a few more components and then test a production build before continuing.

@naomatheus
Copy link
Collaborator Author

Posting this here so I can reference it easily

25 results - 25 files

src/components/button.js:
  2  import PropTypes from "prop-types"
  3: import styled from "styled-components"
  4  

src/components/date-list-hover.js:
   9  import VisuallyHidden from "@reach/visually-hidden"
  10: import styled from "styled-components"
  11  import { colors } from "../theme"

src/components/footer.js:
  4  import { StaticImage } from "gatsby-plugin-image"
  5: import styled from "styled-components"
  6  import { NEGATIVE } from "../utils/constants"

src/components/hero.js:
  3  import { Link } from "gatsby"
  4: import styled from "styled-components"
  5  import { GatsbyImage } from "gatsby-plugin-image"

src/components/modal.js:
  2  import PropTypes from "prop-types"
  3: import styled from "styled-components"
  4  

src/components/cards/card.js:
  2  import PropTypes from "prop-types"
  3: import styled from "styled-components"
  4  import { GatsbyImage, getImage } from "gatsby-plugin-image"

src/components/explore/explore-menu.js:
  2  import PropTypes from "prop-types"
  3: import styled from "styled-components"
  4  import { Link } from "gatsby"

src/components/explore/explore-section.js:
  2  import PropTypes from "prop-types"
  3: import styled from "styled-components"
  4  import {

src/components/explore/filter-by-date.js:
  12  import "react-datepicker/dist/react-datepicker.css"
  13: import styled from "styled-components"
  14  import {

src/components/explore/filter-menu.js:
  10  import VisuallyHidden from "@reach/visually-hidden"
  11: import styled from "styled-components"
  12  

src/components/explore/filter-text-dropdown.js:
  14  } from "@reach/combobox"
  15: import styled from "styled-components"
  16  

src/components/layout/definition-list.js:
  1  import React from "react"
  2: import styled from "styled-components"
  3  import PropTypes from "prop-types"

src/components/layout/layout.js:
  10  import { useStaticQuery, graphql } from "gatsby"
  11: import styled from "styled-components"
  12  

src/components/layout/section.js:
  2  import PropTypes from "prop-types"
  3: import styled from "styled-components"
  4  

src/components/timeline/details.js:
  3  import { graphql, useStaticQuery } from "gatsby"
  4: import styled from "styled-components"
  5  

src/components/timeline/iop-detail.js:
  2  import PropTypes from "prop-types"
  3: import styled from "styled-components"
  4  

src/components/timeline/scroll-shadow.js:
  2  import PropTypes from "prop-types"
  3: import styled, { css } from "styled-components"
  4  

src/components/timeline/timeline-chart.js:
  3  import * as d3 from "d3"
  4: import styled from "styled-components"
  5  

src/pages/faq.js:
  3  import { graphql, Link } from "gatsby"
  4: import styled from "styled-components"
  5  import {

src/pages/explore/upcoming.js:
  1  import React, { useEffect, useState } from "react"
  2: import styled from "styled-components"
  3  import PropTypes from "prop-types"

src/templates/campaign/focus-section.js:
  3  import { graphql, Link } from "gatsby"
  4: import styled from "styled-components"
  5  

src/templates/campaign/hero.js:
  3  import { graphql, Link } from "gatsby"
  4: import styled from "styled-components"
  5  import { GatsbyImage } from "gatsby-plugin-image"

src/templates/instrument/entities.js:
  3  import { graphql } from "gatsby"
  4: import styled from "styled-components"
  5  

src/theme/style-helpers.js:
  1: import { css } from "styled-components"
  2  

test/jest.setup.js:
  1: import "jest-styled-components"

@naomatheus
Copy link
Collaborator Author

Still making progress on this. But ran into the following:https://www.gatsbyjs.com/blog/how-to-use-function-props-with-gatsbys-slice-api/

For reference @Tammo-Feldmann
From the docs - Slicing the components which have action props, like Button, involves an alternative means of passing the function via React context for the slice to consume from there.

@Tammo-Feldmann
Copy link
Contributor

@naomatheus that's a good call. I remember seeing in the Gatsby docs that the slice api does not allow function props. Glad to see that React context will still work in this case. Seems like an annoying refactor rather than a show stopper. Let me know how this goes for you.

@naomatheus
Copy link
Collaborator Author

I've paused on this refactor of the Button component. I found the refactor is a bit more extensive than I am knowledgeable of React Context within Gatsby. So I paused on that, but saved my work in a stash.
I've kept going with the more straight forward components.

@naomatheus
Copy link
Collaborator Author

This is coming along still. I'm working on getting passing tests now.
I decided to rule out any extensive refactors right now. The components that accept function props (Button, Card, and possibly others) I have left out of Slices for now. The refactor required wasn't as straight forward as it seems.
So now I am just getting this PR up to code standards.
In between I may cancel any tests so that they don't have open TCP sockets that keep to test running in perpetuity

@Tammo-Feldmann
Copy link
Contributor

@naomatheus happy to hear that this is working out for you. Leaving the components with setter functions out for now sounds like a good plan. We can always revisit them later if we see that we're getting good gains from the slices that you have separated out so far. Looking forward to seeing how this all shakes out in the end.

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.

2 participants