prismicio / gatsby-source-prismic-graphql

Gatsby source plugin for Prismic GraphQL
MIT License
17 stars 14 forks source link

feat(withpreviewcomponent): component that just renders its children #27

Closed matthewtgilbride closed 3 years ago

matthewtgilbride commented 4 years ago

@MarcMcIntosh @ohlr @lucashfreitas @Floriferous this is what I wrote in my own project...seems simpler...

usage:

export const Homepage: FC = () => {
  const data = useStaticQuery(homepageQuery);
  return (
    <WithPreview query={homepageQuery} data={data}>
      <MyComponent
        data={data}
      />
    </WithPreview>
  );
};

thoughts?

lucashfreitas commented 4 years ago

That's awesome @matthewtgilbride, great work.

Will have a look at the code ASAP. Have you managed to get the preview working use this approach? The plugin doesn't support static query by default. Your solution seems to replicate/reutilize the graphql query rather than reutilizing the static query hooks.

It seems great and simple and could be a way to get the preview working without having to restructure your component if you use static query. But it's important to remember that many projects use this approach:

const useMyCustomDataQuery = () => {
  const myData = useStaticQuery(graphql`
    query {
      allPrismicData {
        edges {
          node {
            ...MyFragment
          }
        }
      }
    }
  `)
  return myData.allPrismicData
}
export default useMyCustomDataQuery

Later on, you import your custom query at your component.

const MyComponent = ( ) => {

const myData = useMyCustomQuery()

For this situation, we would still have to transfer the query directly to our pages. There is no way to get the preview feature working unless you set your graphql queries at the page level, either using the standar export const query, or using gatsby <StaticQuery> and withPreview function and we have now another example provided by your pull request.

Please let me know your thoughts! :)

I am thinking to set up a discord channel, it could be so helpful for other people facing the same issue that we've to spend much time figuring things out due to lacking of documentation/support.

matthewtgilbride commented 4 years ago

Hi @lucashfreitas ,

I have confirmed my solution works with useStaticQuery, but only on development builds, and in my project it also causes a console error to be thrown on production builds (again - the site works but previews do not - as I had asked in #26).

If you use the StaticQuery HOC, everything works (including previews on production builds). I have confirmed this is also the case with the existing withPreview implementation.

So clearly there is some gotcha associated with gatsby build and the whole issue around the graphql function, and this change doesn't address that.

The reason I prefer the approach in this PR is really down to personal preference. I find the second alternative easier to read:

withPreview.tsx

<StaticQuery
  query={`${homepageQuery}`}
  render={
    withPreview(data => (
      <MyComponent data={data} />
    ),
    homepageQuery
  }
/>

WithPreviewComponent.tsx

<StaticQuery
  query={`${homepageQuery}`}
  render={data => (
    <WithPrismicPreview query={homepageQuery} data={data}>
      <MyComponent
        data={data}
      />
    </WithPrismicPreview>
  )}
/>

I think a discord or slack workspace makes a lot of sense. While I am at the beginning of a project and don't have existing code that has to be converted, I would love to have the freedom to write custom hooks as you describe.

I am also happy to hop on the phone to talk more about this. I am still quite new to graphql and gatsby. I have trouble grasping the "set your graphql queries at the page level" limitations and wish there were at least some eslint rule to help us understand when we might be shooting ourselves in the foot. If you would be willing to enlighten me, I would be much obliged! Perhaps in return I can find a way to support the hook ;)

MarcMcIntosh commented 4 years ago

@matthewtgilbride @lucashfreitas isUseStaticQuery broken again?

Would you be able to add some documentation on the component and an example?

matthewtgilbride commented 4 years ago

@matthewtgilbride @lucashfreitas isUseStaticQuery broken again?

@MarcMcIntosh from what I can tell, this is the situation:

I suspect the problem is related to this: https://www.gatsbyjs.org/docs/use-static-query/#known-limitations, however I do not understand the plugin or those limitations well enough to understand exactly what is going on.

I can rebase this branch now that my typescript example is merged, update that example, and add some docs 👍

lucashfreitas commented 4 years ago

@MarcMcIntosh not too sure. The last time (3 weeks ago) I tried to use it it didn't work. As I didn't have time to investigate it I switched my queries to page queries instead.

Please let me know if you need any help to investigate this!

matthewtgilbride commented 4 years ago

@MarcMcIntosh @lucashfreitas I just tried with the latest version (3.6.11) and still face the same issues with production builds and the hook.

I don't think this change provides much value if we can't use the hook. If we can - here is why I think it is a bit nicer:

withPreview.tsx

/**
 * render && render(data) is a bit awkward
 */
export const Homepage: FC = () => {
  const data = useStaticQuery(homepageQuery);
  const render = withPreview(
    data => (
      <MyComponent
        data={data}
      />
    ),
    homepageQuery,
  );
  return render && render(data);
};

WIthPreviewComponent.tsx

/**
 * just wrap your component, which can take whatever props it wants
 */
export const Homepage: FC = () => {
  const data = useStaticQuery(homepageQuery);
  return (
    <WithPrismicPreview query={homepageQuery} data={data}>
      <MyComponent
        data={data}
      />
    </WithPrismicPreview>
  );
};

I suggest we hold off on merging this until we either have a fix, or have updated the docs.

MarcMcIntosh commented 4 years ago

@matthewtgilbride @lucashfreitas

Yes, usually when static query stops working it's due to the pathced version of babel-plugin-remove-graphql-queries.js being out of sync with changes made to the original version in Gatsby.

MarcMcIntosh commented 4 years ago

@matthewtgilbride could you add some documentation? :)

matthewtgilbride commented 4 years ago

@matthewtgilbride could you add some documentation? :)

@MarcMcIntosh I'm not sure how much value this PR provides, especially if the hook still isn't working. Is that the case?

As I mentioned in another issue, I've abandoned Gatsby for NextJS...my apologies for the delayed responses.

MarcMcIntosh commented 4 years ago

@matthewtgilbride yes useStaticQuery should be working.