graysonhicks / gatsby-plugin-remote-images

:floppy_disk: A Gatsby.js plugin for downloading and linking remote images from another node's field for the benefits of gatsby-image.
155 stars 39 forks source link

Broken with Gatsby 2.18.0 #28

Closed wKovacs64 closed 4 years ago

wKovacs64 commented 5 years ago

Something seems to have changed in Gatsby 2.17.17 -> 2.18.0 that impacts this plugin. I'm no longer seeing the node created. Confirmed missing via GraphiQL, and the build error is as follows:

 ERROR #85907  GRAPHQL

There was an error in your GraphQL query:

- Unknown field 'siteImageFile' on type 'Site'.

CodeSandbox Reproduction

avwie commented 5 years ago

I was pulling my hair out, because I have the exact same thing. My localImage node is not showing anymore. This broke my complete build.

Zebusch commented 5 years ago

Seeing the same thing. Downgrading to 2.17, it works just fine.

wKovacs64 commented 5 years ago

This seems related to https://github.com/gatsbyjs/gatsby/pull/19092. I did some debugging of this plugin and node has the new myImageFile___NODE field on it at the end of onCreateNode, so Gatsby must be stripping or ignoring it or something?

cc @vladar

vladar commented 5 years ago

Sorry to see this change broke this plugin. But looks like this plugin is doing direct node mutation in onCreateNode which is not supported by Gatsby. Happens here:

https://github.com/graysonhicks/gatsby-plugin-remote-images/blob/649abaa2e88f3c5c5f0e27c2f9cb7bea503b7dc5/src/gatsby-node.js#L72-L76

There is a separate action for situations when you need adding a field to a node: createNodeField. A quote from docs:

Also since nodes are immutable, you can’t mutate the node directly.
So to extend another node, use this.

But a better way to declare a foreign key relationship on a node would be using schema customization APIs (Gatsby will probably shift towards this way of customizing types in future and deprecate old inference tricks eventually). I think createResolvers could be the way to go. Something along the lines:

exports.createResolvers = ({ createResolvers }) => {
  const resolvers = {
    [nodeType]: {
      [fieldName]: {
        type: "File",
        resolve(source, args, context, info) {
          return context.nodeModel.getNodeById(source.parentNodeId)
        },
      },
    },
  }
  createResolvers(resolvers)
}

Hope it helps!

jbolda commented 5 years ago

@vladar I don't know that the direct node manipulation is necessarily the issue the root issue. I am also getting errors related to the gatsby-source-filesystem function createRemoteFileNode in the gatsby-source-airtable plugin. I am getting - Unknown field 'Attachments' on type 'AirtableData'. where the plugin does not create this type. The plugin does use the ___NODE (without direct node manipulation because it is a source plugin)/.

Related PR that breaks on ^2.18.0: https://github.com/jbolda/gatsby-source-airtable/pull/117 Job line error: https://travis-ci.com/jbolda/gatsby-source-airtable/jobs/259371321#L510

vladar commented 5 years ago

@jbolda Your link points to localFiles___NODE on AirtableField but the error is about Attachments on type AirtableData.

I did a quick sanity check - added:

console.log(`create node `, row.id ,` data: `, Object.keys(node.data))

before this createNode call (which will create Airtable and AirtableData types).

And turns out that the field Attachments is not there (see the log in gist). It is added somewhere else after the createNode call is processed by Gatsby. Maybe via mutation here or here.

There might be some race condition that didn't manifest itself before the schema rebuilding PR but shows up now.

So to summarize this - it is likely a mutation issue. Does it help?

kpennell commented 5 years ago

whew! Downgrading helped.

For reference

gatsby-config.js


const dotenv = require("dotenv")

dotenv.config({
  path: `.env.${process.env.NODE_ENV}`,
})

module.exports = {
  siteMetadata: {
    title: `AcroTags`,
    description: "Demo site for Gatsby AcroTags Blog",
    author: `Kyle Pennell`,
  },
  plugins: [

    'gatsby-plugin-sharp',
    'gatsby-transformer-sharp',
    `gatsby-plugin-react-helmet`,

    {
      // keep as first gatsby-source-filesystem plugin for gatsby image support
      resolve: 'gatsby-source-filesystem',
      options: {
        path: `${__dirname}/static/img`,
        name: 'uploads',
      },
    },
    {
      resolve: 'gatsby-source-filesystem',
      options: {
        path: `${__dirname}/src/pages`,
        name: 'pages',
      },
    },
    {
      resolve: "gatsby-plugin-remote-images",
      options: {
        nodeType: "item",
        imagePath: "thumbnail",
        name: 'optimized_thumbnail',
      }
    },
    {
      resolve: "gatsby-plugin-remote-images",
      options: {
        nodeType: "item",
        imagePath: "instructor_image", 
        name: 'optimized_instructor_image',
      }
    },
    {
      resolve: `gatsby-plugin-manifest`,
      options: {
        name: `gatsby-starter-default`,
        short_name: `starter`,
        start_url: `/`,
        background_color: `#663399`,
        theme_color: `#663399`,
        display: `minimal-ui`,

      },
    },
    {
      resolve: `gatsby-plugin-material-ui`

    },
    // this (optional) plugin enables Progressive Web App + Offline functionality
    // To learn more, visit: https://gatsby.dev/offline
    // 'gatsby-plugin-offline',
  ],
}

gatsby-node.js

const path = require(`path`)
const axios = require("axios")
const _ = require("lodash")

exports.createPages = ({ actions, graphql }) => {
  const { createPage } = actions

  return graphql(`
    {
      allItem {
        edges {
          node {
            id
            move
            videoUrl
            thumbnail
            title
            tags

            instructor
            instructor_image
            description
            videoId
            profileUrl
            user_id

          }
        }
      }
    }
  `).then(result => {
    if (result.errors) {
      result.errors.forEach(e => console.error(e.toString()))
      return Promise.reject(result.errors)
    }

    const items = result.data.allItem.edges

    items.forEach(edge => {
      const id = edge.node.id
      const title = edge.node.title
      const videoPath = `/video/${_.kebabCase(title)}/`

      createPage({
        path: videoPath,
        component: path.resolve(`src/templates/single-item.js`),
        context: {
          itemId: id,
        },
      })
    })

    // Tag pages:
    let tags = []

    items.forEach(item => {
      if (item.node.tags.length > -1) {
        tags = tags.concat(item.node.tags)
      }
    })

    tags = _.uniq(tags)

    tags.forEach(tag => {
      const tagPath = `/tag/${_.kebabCase(tag)}/`

      createPage({
        path: tagPath,
        component: path.resolve(`src/templates/single-tag.js`),
        context: {
          tag,
        },
      })
    })

    let instructors = []

    items.forEach(item => {
      if (item.node.instructor.length > -1) {
        instructors = instructors.concat(item.node.instructor)
      }
    })

    instructors = _.uniq(instructors)

    instructors.forEach(instructor => {
      const instructorPath = `/instructor/${_.kebabCase(instructor)}/`

      createPage({
        path: instructorPath,
        component: path.resolve(`src/templates/single-instructor.js`),
        context: {
          instructor,
        },
      })
    })
  })
}

exports.sourceNodes = async ({
  actions,
  createNodeId,
  createContentDigest,
}) => {
  const { createNode } = actions

  const fetchFormItems = () =>
    axios.get(
      `json source ---removed...`
    )

  const response = await fetchFormItems()

  const arrayOfItems = response.data.valueRanges[0].values

  let rows = []
  for (var i = 1; i < arrayOfItems.length; i++) {
    var rowObject = {}
    for (var j = 0; j < arrayOfItems[i].length; j++) {
      rowObject[arrayOfItems[0][j]] = arrayOfItems[i][j]
    }
    rows.push(rowObject)
  }

  let itemsArrayWithTagsArray = rows.map(function(item) {
    item.tags = item.tags.split(",").map(item => item.trim())
    item = { ...item }
    return item
  })

  itemsArrayWithTagsArray.map((item, i) => {
    const itemNode = {
      id: createNodeId(`${i}`),
      parent: `__SOURCE__`,
      internal: {
        type: `item`, // name of the graphQL query --> allItem {}
        contentDigest: createContentDigest(item),
      },
      children: [],
      move: item.move,
      videoUrl: item.videoUrl,
      thumbnail: item.thumbnail,
      title: item.title,
      tags: item.tags,
      pubDate: item.pubDate,

      instructor: item.instructor,
      instructor_image: item.instructor_image,
      description: item.description,
      videoId: item.videoId,

    }

    createNode(itemNode)
  })
}

Not sure if that helps. I'm super grateful for this plugin.

jbolda commented 5 years ago

@vladar It looks like it was actually the Attachments returning as a promise which prior gatsby would let resolve, but now awaiting that promise fixed the bug. The mutations you pointed out are just local to those functions and prior to the node being created, so it doesn't seem that was the issue.

ref: https://github.com/jbolda/gatsby-source-airtable/pull/119/commits/7d84a2d45a69fd79bbb2d0a706274879b7f140f9

Looking at this plugin, it does make the mutations directly on the node, as pointed out, which is likely the issue here. All promises areawaited in this plugin. So it seems the two issues are unrelated.

lawrenceong001 commented 5 years ago

hi totally new to gatsby -- just out of curiosity how do I downgrade back to 2.7? I tried

npm unnistall -g gatsby-cli npm i -g gatsby-cli@2.7.59

on my site, i still end up with 2.8.14

tks

ooloth commented 5 years ago

You’re changing the gatsby installation that lives globally on your computer (that’s what the -g flag does).

Instead, you want to change the gatsby installation specific to your project.

So, navigate your console to your project folder and run npm i gatsby@2.17.17 (no need to uninstall first).

lawrenceong001 commented 5 years ago

thanks @ooloth! that solved my image issue.

tks Lawrence

wKovacs64 commented 5 years ago

You can also update to gatsby@2.18.5, as a "fix" has landed there (actually it just re-hides the problem, as it was hidden prior to 2.18.0). This plugin (and others) still needs updating, but the problem is masked in the latest version of Gatsby.

lawrenceong001 commented 5 years ago

thanks for the suggestion @wKovacs64. I will look into this after I run through my work with the project.

tks Lawrence

btiwaree commented 5 years ago

just spent a good 4 hours trying to figure out what was wrong with my local plugin that used gatsby-plugin-remote-images 😢, thanks a lot @wKovacs64 for this.

graysonhicks commented 5 years ago

Sorry all, for some reason I was not getting my Github notifications on this! 😅 If there is a PR that can improve this plugin to make the move long term to use the createResolvers method, I am happy to accept.

@vladar @wKovacs64

graysonhicks commented 4 years ago

You can also update to gatsby@2.18.5, as a "fix" has landed there (actually it just re-hides the problem, as it was hidden prior to 2.18.0). This plugin (and others) still needs updating, but the problem is masked in the latest version of Gatsby.

Long term fix in PR here by @wKovacs64 .