imgix / gatsby

A simple yet powerful integration between Gatsby and imgix
BSD 2-Clause "Simplified" License
30 stars 6 forks source link

Gatsby v4 compatibility #147

Closed finematterdave closed 2 years ago

finematterdave commented 2 years ago

After upgrading to gatsby 4 and 1.7.0 of the imgix library (which adds gatsby 4 support), I get errors related to invalid config formats, though the format hasn't changed (and still seems to match the readme)

Steps to reproduce the behaviour:

  1. Have the following section in your fields config in gatsby-config.js:
fields: [
        {
          nodeType: "Instagram",
          fieldName: "image_imgix",
          getURL: (node) => node.image?.url
        },
        {
          nodeType: "ShopifyProductImages",
          fieldName: "imgix",
          getURL: (node) => node.url 
        },
      ],
  1. Build with production node env
  2. Error during "Validating Rendering Engines" of gatsby build:
    error [@imgix/gatsby] Fatal error during setup: Error: The plugin config is not in the correct format. Errors: Invalid value [{"nodeType":"Instagram","fieldName":"image_imgix"},{"nodeType":"ShopifyProductImages","fieldName":"imgix"}] supplied to : GatsbySourceUrlOptions/fields: (undefined | Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>)/0: undefined,Invalid value undefined supplied to : GatsbySourceUrlOptions/fields: (undefined | Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>)/1: Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>/0: (({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))/0: ({ nodeType: string, fieldName: string } & { getURL: Function })/1: { getURL: Function }/getURL: Function,Invalid value undefined supplied to : GatsbySourceUrlOptions/fields: (undefined | Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>)/1: Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>/0: (({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))/1: ({ nodeType: string, fieldName: string } & { getURLs: Function })/1: { getURLs: Function }/getURLs: Function,Invalid value undefined supplied to : GatsbySourceUrlOptions/fields: (undefined | Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>)/1: Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>/1: (({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))/0: ({ nodeType: string, fieldName: string } & { getURL: Function })/1: { getURL: Function }/getURL: Function,Invalid value undefined supplied to : GatsbySourceUrlOptions/fields: (undefined | Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>)/1: Array<(({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))>/1: (({ nodeType: string, fieldName: string } & { getURL: Function }) | ({ nodeType: string, fieldName: string } & { getURLs: Function }))/1: ({ nodeType: string, fieldName: string } & { getURLs: Function })/1: { getURLs: Function }/getURLs: Function

Information:

angeloashmore commented 2 years ago

Potentially related: https://github.com/gatsbyjs/gatsby/discussions/33199#discussioncomment-1345213

finematterdave commented 2 years ago

Does seem like it's very similar... @frederickfogerty happy to PR something if you've got an approach that'd make sense for this plugin

frederickfogerty commented 2 years ago

Hey @finematterdave, thanks for the offer. At this stage I don't exactly know what is going here - will take a look at this next week and give an update then.

finematterdave commented 2 years ago

Hey @frederickfogerty , sorry to bug you, did you get a chance to look at this?

frederickfogerty commented 2 years ago

Hey @finematterdave thanks for following up! Unfortunately this one slipped through the cracks - I'll make sure I get onto this next week (hopefully Tuesday)

frederickfogerty commented 2 years ago

Hi @finematterdave, I looked into this today and I can't seem to reproduce your issue:

image

Dependencies

  "dependencies": {
    "@imgix/gatsby": "1.7.0",
    "gatsby": "^4.3.0",
    "gatsby-plugin-image": "^2.4.0",
    "react": "^17.0.1",
    "react-dom": "^17.0.1"
  }

If this issue is still persisting for you, could post more of your gatsby-config.js, the specific commands use to trigger the issue, your dependency versions, and if possible an example repo which is showing the issue?

finematterdave commented 2 years ago

Hey @frederickfogerty , apologies, I shouldn't have said "build" in my original issue, looking closer this seems to only happen when using the gatsby develop or gatsby serve commands when we try and run some automated tests in the CI. Let me know if it happens for you with develop instead and I'll work on a basic repo to share as well

As @angeloashmore mentioned it seems similar to https://github.com/gatsbyjs/gatsby/discussions/33199#discussioncomment-1345213

finematterdave commented 2 years ago

Actually getting a test repo up and running is painful due to https://github.com/imgix/gatsby/pull/155 not being released yet, once that is I'll set something up with the quick start gatsby repo (all their stuff is on the newer versions now, so gets into some dependency issues)

frederickfogerty commented 2 years ago

Unfortunately, I also couldn't repro it with the develop command.

I released #155 in 1.7.4 just now - let me know if that helps you.

finematterdave commented 2 years ago

https://github.com/finematterdave/example-imgix-issue is a broken example, clone, run npm install then npm run build, let me know if it's also broken for you... hope so!

frederickfogerty commented 2 years ago

@finematterdave just wanted to follow up here that I'm still investigating this. I hope to have another update for you in the coming days

finematterdave commented 2 years ago

No problem, thanks @frederickfogerty, let me know if I can help at all

frederickfogerty commented 2 years ago

Hi @finematterdave, I'm still investigating this. At least this time I've managed to reproduce the issue! Will let you know how I go

frederickfogerty commented 2 years ago

Turns out @angeloashmore was totally correct, it is related to https://github.com/gatsbyjs/gatsby/discussions/33199#discussioncomment-1345213. This took a long day of debugging and searching to find out what was going wrong. I'm going to wait for Angelo to update me about any fixes he made, but at this I don't have an immediate fix.

angeloashmore commented 2 years ago

Hey @frederickfogerty, I replied to your comment in the Gatsby discussion. Let me know if you have any questions. 👍

frederickfogerty commented 2 years ago

Thanks @angeloashmore - replied over there too

finematterdave commented 2 years ago

Hey @frederickfogerty , any thoughts on an approach here?

frederickfogerty commented 2 years ago

Hi @finematterdave. We are going to rework the configuration of this plugin to not use a function option, and instead use a string option for the getURL config option. I have been slammed both on vacation a lot recently and lots of other high priority work has come up so I haven't been able to get to this, but if you feel like opening a PR for this change, that would be welcome. Otherwise, I hope to be able to get to this next week or the week after.

finematterdave commented 2 years ago

No worries @frederickfogerty , I probably won't get time to get to it before you do, but the approach sounds sensible! Hope you had a nice break 🏝

frederickfogerty commented 2 years ago

Hi @finematterdave, I just released this onto our prerelease channel - would you be able to help us test this out? 🙌 You can install the prelease channel with yarn add @imgix/gatsby@next

imgix-git-robot commented 2 years ago

:tada: This issue has been resolved in version 1.7.6-rc.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

finematterdave commented 2 years ago

Hey @frederickfogerty , works great for me, readme instructions were nice and clear too, good work! 🎉

frederickfogerty commented 2 years ago

Hey @finematterdave just wanted to follow up here and see how this was going for you and if you had any issues? If it's been smooth, then we'll promote this to the stable channel

finematterdave commented 2 years ago

Yep all good for us, been working fine and haven't found any strange issues

frederickfogerty commented 2 years ago

Fixed by #167 and #163 💯

imgix-git-robot commented 2 years ago

:tada: This issue has been resolved in version 1.7.6 :tada:

The release is available on:

Your semantic-release bot :package::rocket: