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

Fix gatsby-plugin-image webpack issue with global #30658

Closed

Conversation

kelvindecosta
Copy link
Contributor

Overview

The gatsby-plugin-image plugin doesn't work nicely with plugins that rely on the global variable.
This pull request attempts to address it.

Problem

The plugin adds the following webpack configuration:

export const onCreateWebpackConfig: GatsbyNode["onCreateWebpackConfig"] = ({
stage,
plugins,
actions,
}) => {
if (
stage !== `develop` &&
stage !== `build-javascript` &&
stage !== `build-html`
) {
return
}
actions.setWebpackConfig({
plugins: [
plugins.define({
[`global.GATSBY___IMAGE`]: true,
}),
],
})
}

This causes the following error

  Uncaught ReferenceError: global is not defined

Solution

I think that the [global.GATSBY___IMAGE] key is causing the issue.

I've looked at other official Gatsby plugins that use the plugins.define() method to define global variables,
and haven't come across a key that matches this pattern.

Essentially, [global.GATSBY___IMAGE] is changed to GATSBY_IMAGE.

Testing

With the changes made, the tests are unaffected,
i.e., the same tests that failed prior to the changes, fail now as well:

Test Suites: 1 failed, 4 passed, 5 total
Tests:       7 failed, 57 passed, 64 total
Snapshots:   16 passed, 16 total
Time:        7.616s
Ran all test suites.
Full Test Log
yarn run v1.21.0
$ jest
 PASS  src/__tests__/babel-utils.ts
 PASS  src/__tests__/image-utils.ts
 PASS  src/components/__tests__/hooks.ts
 PASS  src/components/__tests__/gatsby-image.server.tsx
  ● Console

    console.error ../../node_modules/prop-types/checkPropTypes.js:20
      Warning: Failed prop type: The prop `src` is marked as required in `Image`, but its value is `undefined`.
          in Image (created by Picture)
          in Picture (created by MainImage)
          in MainImage (created by GatsbyImage)
          in LayoutWrapper (created by GatsbyImage)
          in div (created by GatsbyImageHydrator)
          in GatsbyImageHydrator (created by GatsbyImage)
          in GatsbyImage

 FAIL  src/components/__tests__/gatsby-image.browser.tsx
  ● Console

    console.error node_modules/@testing-library/react/dist/act-compat.js:52
      Warning: Failed prop type: The prop `image` is marked as required in `GatsbyImage`, but its value is `undefined`.
          in GatsbyImage
    console.error node_modules/@testing-library/react/dist/act-compat.js:52
      Warning: Failed prop type: The "alt" prop is required in GatsbyImage. If the image is purely presentational then pass an empty string: e.g. alt="". Learn more: https://a11y-style-guide.com/style-guide/section-media.html
          in GatsbyImage
    console.error node_modules/@testing-library/react/dist/act-compat.js:52
      Warning: Did not expect server HTML to contain the text node "
            " in <div>.
    console.error node_modules/@testing-library/react/dist/act-compat.js:52
      Warning: Unknown event handler property `onStartLoad`. It will be ignored.
          in img (created by Image)
          in Image (created by Picture)
          in Picture (created by MainImage)
          in MainImage
          in LayoutWrapper

  ● GatsbyImage browser › shows a suggestion to switch to the new gatsby-image API when available

    TypeError: MutationObserver is not a constructor

      85 |     )
      86 |
    > 87 |     await waitFor(() => container.querySelector(`[data-placeholder-image=""]`))
         |           ^
      88 |
      89 |     expect(console.warn).toBeCalledWith(
      90 |       `[gatsby-plugin-image] You're missing out on some cool performance features. Please add "gatsby-plugin-image" to your gatsby-config.js`

      at node_modules/@testing-library/dom/dist/wait-for.js:85:18
      at waitFor (node_modules/@testing-library/dom/dist/wait-for.js:40:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:167:54
      at node_modules/@testing-library/react/dist/pure.js:57:22
      at node_modules/@testing-library/react/dist/act-compat.js:60:24
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:24386:12)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1092:14)
      at node_modules/@testing-library/react/dist/act-compat.js:59:20
      at asyncAct (node_modules/@testing-library/react/dist/act-compat.js:38:14)
      at Object.asyncWrapper (node_modules/@testing-library/react/dist/pure.js:56:35)
      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:167:35)
      at Object.<anonymous> (src/components/__tests__/gatsby-image.browser.tsx:87:11)

  ● GatsbyImage browser › shows nothing when the image props is not passed

    TypeError: MutationObserver is not a constructor

      100 |     const { container } = render(<GatsbyImageAny />)
      101 |
    > 102 |     await waitFor(() => container.querySelector(`[data-placeholder-image=""]`))
          |           ^
      103 |
      104 |     expect(console.warn).toBeCalledWith(
      105 |       `[gatsby-plugin-image] Missing image prop`

      at node_modules/@testing-library/dom/dist/wait-for.js:85:18
      at waitFor (node_modules/@testing-library/dom/dist/wait-for.js:40:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:167:54
      at node_modules/@testing-library/react/dist/pure.js:57:22
      at node_modules/@testing-library/react/dist/act-compat.js:60:24
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:24386:12)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1092:14)
      at node_modules/@testing-library/react/dist/act-compat.js:59:20
      at asyncAct (node_modules/@testing-library/react/dist/act-compat.js:38:14)
      at Object.asyncWrapper (node_modules/@testing-library/react/dist/pure.js:56:35)
      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:167:35)
      at Object.<anonymous> (src/components/__tests__/gatsby-image.browser.tsx:102:11)

  ● GatsbyImage browser › cleans up the DOM when unmounting

    TypeError: MutationObserver is not a constructor

      115 |     )
      116 |
    > 117 |     await waitFor(() => container.querySelector(`[data-placeholder-image=""]`))
          |           ^
      118 |
      119 |     unmount()
      120 |

      at node_modules/@testing-library/dom/dist/wait-for.js:85:18
      at waitFor (node_modules/@testing-library/dom/dist/wait-for.js:40:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:167:54
      at node_modules/@testing-library/react/dist/pure.js:57:22
      at node_modules/@testing-library/react/dist/act-compat.js:60:24
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:24386:12)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1092:14)
      at node_modules/@testing-library/react/dist/act-compat.js:59:20
      at asyncAct (node_modules/@testing-library/react/dist/act-compat.js:38:14)
      at Object.asyncWrapper (node_modules/@testing-library/react/dist/pure.js:56:35)
      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:167:35)
      at Object.<anonymous> (src/components/__tests__/gatsby-image.browser.tsx:117:11)

  ● GatsbyImage browser › does nothing on first server hydration

    TypeError: MutationObserver is not a constructor

      133 |     )
      134 |
    > 135 |     const placeholder = await waitFor(() =>
          |                               ^
      136 |       container.querySelector(`[data-placeholder-image=""]`)
      137 |     )
      138 |     const mainImage = container.querySelector(`[data-main-image=""]`)

      at node_modules/@testing-library/dom/dist/wait-for.js:85:18
      at waitFor (node_modules/@testing-library/dom/dist/wait-for.js:40:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:167:54
      at node_modules/@testing-library/react/dist/pure.js:57:22
      at node_modules/@testing-library/react/dist/act-compat.js:60:24
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:24386:12)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1092:14)
      at node_modules/@testing-library/react/dist/act-compat.js:59:20
      at asyncAct (node_modules/@testing-library/react/dist/act-compat.js:38:14)
      at Object.asyncWrapper (node_modules/@testing-library/react/dist/pure.js:56:35)
      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:167:35)
      at Object.<anonymous> (src/components/__tests__/gatsby-image.browser.tsx:135:31)

  ● GatsbyImage browser › relies on native lazy loading when the SSR element exists and that the browser supports native lazy loading

    TypeError: MutationObserver is not a constructor

      161 |     )
      162 |
    > 163 |     const img = await waitFor(() =>
          |                       ^
      164 |       container.querySelector(`[data-main-image=""]`)
      165 |     )
      166 |

      at node_modules/@testing-library/dom/dist/wait-for.js:85:18
      at waitFor (node_modules/@testing-library/dom/dist/wait-for.js:40:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:167:54
      at node_modules/@testing-library/react/dist/pure.js:57:22
      at node_modules/@testing-library/react/dist/act-compat.js:60:24
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:24386:12)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1092:14)
      at node_modules/@testing-library/react/dist/act-compat.js:59:20
      at asyncAct (node_modules/@testing-library/react/dist/act-compat.js:38:14)
      at Object.asyncWrapper (node_modules/@testing-library/react/dist/pure.js:56:35)
      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:167:35)
      at Object.<anonymous> (src/components/__tests__/gatsby-image.browser.tsx:163:23)

  ● GatsbyImage browser › relies on intersection observer when the SSR element is not resolved

    TypeError: MutationObserver is not a constructor

      186 |     )
      187 |
    > 188 |     await waitFor(() => container.querySelector(`[data-main-image=""]`))
          |           ^
      189 |
      190 |     expect(onStartLoadSpy).toBeCalledWith({ wasCached: false })
      191 |   })

      at node_modules/@testing-library/dom/dist/wait-for.js:85:18
      at waitFor (node_modules/@testing-library/dom/dist/wait-for.js:40:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:167:54
      at node_modules/@testing-library/react/dist/pure.js:57:22
      at node_modules/@testing-library/react/dist/act-compat.js:60:24
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:24386:12)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1092:14)
      at node_modules/@testing-library/react/dist/act-compat.js:59:20
      at asyncAct (node_modules/@testing-library/react/dist/act-compat.js:38:14)
      at Object.asyncWrapper (node_modules/@testing-library/react/dist/pure.js:56:35)
      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:167:35)
      at Object.<anonymous> (src/components/__tests__/gatsby-image.browser.tsx:188:11)

  ● GatsbyImage browser › relies on intersection observer when browser does not support lazy loading

    TypeError: MutationObserver is not a constructor

      204 |     )
      205 |
    > 206 |     await waitFor(() => container.querySelector(`[data-main-image=""]`))
          |           ^
      207 |
      208 |     expect(onStartLoadSpy).toBeCalledWith({ wasCached: false })
      209 |   })

      at node_modules/@testing-library/dom/dist/wait-for.js:85:18
      at waitFor (node_modules/@testing-library/dom/dist/wait-for.js:40:10)
      at node_modules/@testing-library/dom/dist/wait-for.js:167:54
      at node_modules/@testing-library/react/dist/pure.js:57:22
      at node_modules/@testing-library/react/dist/act-compat.js:60:24
      at batchedUpdates$1 (../../node_modules/react-dom/cjs/react-dom.development.js:24386:12)
      at act (../../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1092:14)
      at node_modules/@testing-library/react/dist/act-compat.js:59:20
      at asyncAct (node_modules/@testing-library/react/dist/act-compat.js:38:14)
      at Object.asyncWrapper (node_modules/@testing-library/react/dist/pure.js:56:35)
      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:167:35)
      at Object.<anonymous> (src/components/__tests__/gatsby-image.browser.tsx:206:11)

Test Suites: 1 failed, 4 passed, 5 total
Tests:       7 failed, 57 passed, 64 total
Snapshots:   16 passed, 16 total
Time:        7.616s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I am not sure how to build the plugin and use it with my site with these new changes.
Hence, I haven't tested if the changes actually fix the issue.

Conclusion

I think that since the changes made to the plugin
keep it consistent with other official plugins that require global variables,
it should fix the issue.

A few changes to the codebase have swapped global for window.
This might cause a few issues, and require review.

I have not done any rigorous testing other than running the tests before and after the changes.

Any suggestions are welcome!
I would love some feedback about this fix and where it could be improved.

Related Issues

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Apr 2, 2021
@LekoArts LekoArts added topic: media Related to gatsby-plugin-image, or general image/media processing topics and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Apr 6, 2021
@ascorbic
Copy link
Contributor

ascorbic commented Apr 6, 2021

Hi. This will break sites, because you are trying to access window in SSR. The solution is to not try and access it on window, or anything, but rather to just access it as a global.

@LekoArts
Copy link
Contributor

LekoArts commented Apr 6, 2021

Closing in favor of #30713 - thanks for the PR though!

@LekoArts LekoArts closed this Apr 6, 2021
@kelvindecosta
Copy link
Contributor Author

Thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: media Related to gatsby-plugin-image, or general image/media processing topics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants