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): fix image flickers #35226

Merged
merged 20 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions e2e-tests/contentful/cypress/integration/rich-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ function testWithGatsbyPluginImage(elem) {
const cleanHtml = html
.replace(base64ImageExp, `data:image/redacted;base64,redacted`)
.replace(styleAttrExp, ``)
.replace(/data-gatsby-image-ssr=\"\"/gm, "")

// Create a DOM element with the redacted base64 data
cy.document().then(document => {
Expand Down
39 changes: 19 additions & 20 deletions e2e-tests/contentful/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,30 @@
"author": "Kyle Mathews <[email protected]>",
"dependencies": {
"@contentful/rich-text-types": "^14.1.2",
"cypress": "^6.8.0",
"cypress-image-snapshot": "^4.0.1",
"gatsby": "^3.1.1",
"gatsby-image": "^3.3.0",
"gatsby-plugin-image": "^1.3.1",
"gatsby-plugin-sharp": "^3.1.2",
"gatsby-source-contentful": "^5.1.1",
"gatsby-transformer-remark": "^4.0.0",
"gatsby-transformer-sharp": "^3.3.0",
"gatsby-transformer-sqip": "3.3.1",
"gatsby": "next",
"gatsby-image": "next",
"gatsby-plugin-image": "next",
"gatsby-plugin-sharp": "next",
"gatsby-source-contentful": "next",
"gatsby-transformer-remark": "next",
"gatsby-transformer-sharp": "next",
"gatsby-transformer-sqip": "next",
"modern-normalize": "^1.0.0",
"prop-types": "^15.7.2",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"slugify": "^1.5.0"
},
"devDependencies": {
"@cypress/snapshot": "^2.1.7",
"cross-env": "^7.0.3",
"cypress": "^9.5.4",
"cypress-image-snapshot": "^4.0.1",
"gatsby-cypress": "latest",
"prettier": "^2.6.2",
"srcset": "^5.0.0",
"start-server-and-test": "^1.7.1"
},
"keywords": [
"gatsby"
],
Expand All @@ -35,17 +43,8 @@
"cy:open": "cypress open",
"cy:run": "node ../../scripts/cypress-run-with-conditional-record-flag.js --browser chrome"
},
"devDependencies": {
"@cypress/snapshot": "^2.1.7",
"cross-env": "^7.0.3",
"gatsby-cypress": "^1.3.0",
"is-ci": "^3.0.0",
"prettier": "2.2.1",
"srcset": "^5.0.0",
"start-server-and-test": "^1.7.1"
},
"repository": {
"type": "git",
"url": "https://github.com/gatsbyjs/gatsby-starter-default"
}
}
}
6 changes: 3 additions & 3 deletions e2e-tests/contentful/snapshots.js

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
const MutationObserver =
window.MutationObserver || window.WebKitMutationObserver

// helper function to observe DOM changes
function observeDOM(obj, options, callback) {
if (!obj || obj.nodeType !== window.Node.ELEMENT_NODE) {
throw new Error("can not observe this element")
}

const obs = new MutationObserver(callback)

obs.observe(obj, { childList: true, subtree: true, ...options })

return () => {
obs.disconnect()
}
}

describe(`gatsby-plugin-image`, () => {
it(`doesn't recycle image nodes when not necessary`, () => {
const mutationStub = cy.stub()
let cleanup

cy.visit(`/gatsby-plugin-image-page-1/`)

// wait for the image to load
cy.get("[data-main-image]").should("be.visible")

// start watching mutations in the image-wrapper
cy.get("body").then($element => {
cleanup = observeDOM($element[0], {}, mutations => {
const normalizedMutations = []
mutations.forEach(mutation => {
if (
mutation.type === "childList" &&
mutation.target.classList.contains("gatsby-image-wrapper")
) {
normalizedMutations.push({
type: mutation.type,
addedNodes: !!mutation.addedNodes.length,
removedNodes: !!mutation.removedNodes.length,
})
}
})

if (normalizedMutations.length) {
mutationStub(normalizedMutations)
}
})
})

cy.window()
.then(win => win.___navigate(`/gatsby-plugin-image-page-2/`))
.waitForRouteChange()

// wait for the image to load
cy.get("[data-main-image]").should("be.visible")
cy.wait(500)

cy.then(() => {
cleanup()
expect(mutationStub).to.be.calledWith([
{
type: "childList",
addedNodes: true,
removedNodes: false,
},
])
})
})

it(`rerenders when image src changed`, () => {
const mutationStub = cy.stub()
let cleanup

cy.visit(`/gatsby-plugin-image-page-1/`)

// wait for the image to load
cy.get("[data-main-image]").should("be.visible")

// start watching mutations in the image-wrapper
cy.get("#image-wrapper").then($element => {
cleanup = observeDOM($element[0], {}, mutations => {
const normalizedMutations = []
mutationStub(
mutations.map(mutation => {
normalizedMutations.push({
addedNodes: mutation.addedNodes,
removedNodes: mutation.removedNodes,
})

return {
type: mutation.type,
addedNodes: !!mutation.addedNodes.length,
removedNodes: !!mutation.removedNodes.length,
}
})
)

Cypress.log({
name: "MutationObserver",
message: `${normalizedMutations.length} mutations`,
consoleProps: () => {
return {
mutations: normalizedMutations,
}
},
})
})
})

cy.get("#click").click()

cy.wait(500)

cy.get("[data-main-image]", {
timeout: 5000,
}).should("be.visible")

cy.then(() => {
cleanup()
expect(mutationStub).to.be.calledOnce
expect(mutationStub).to.be.calledWith([
{
type: "childList",
addedNodes: true,
removedNodes: true,
},
])
})
})

it(`rerenders when background color changes`, () => {
const mutationStub = cy.stub()
let cleanup

cy.visit(`/gatsby-plugin-image-page-2/`)

// wait for the image to load
cy.get("[data-main-image]").should("be.visible")

// start watching mutations in the image-wrapper
cy.get("#image-wrapper").then($element => {
cleanup = observeDOM(
$element[0],
{
attributes: true,
attributeFilter: ["style"],
},
mutations => {
mutationStub(
mutations.map(mutation => ({
type: mutation.type,
attributeName: mutation.attributeName,
}))
)
}
)
})

cy.get("[data-gatsby-image-wrapper]").should(
"have.css",
"background-color",
"rgb(102, 51, 153)"
)

cy.get("#click").click()

cy.wait(500)

// wait for the image to load
cy.get("[data-main-image]").should("be.visible")

cy.get("[data-gatsby-image-wrapper]").should(
"have.css",
"background-color",
"rgb(255, 0, 0)"
)
cy.then(() => {
cleanup()
expect(mutationStub).to.be.calledOnce
expect(mutationStub).to.be.calledWith([
{
type: "attributes",
attributeName: "style",
},
])
})
})
})
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// TODO: In https://github.com/gatsbyjs/gatsby/pull/35226 the skip needs to be removed

describe.skip(`Production build tests`, () => {
describe(`Production build tests`, () => {
it(`should remount when navigating to different template`, () => {
cy.visit(`/`).waitForRouteChange()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { graphql } from "gatsby"
import React from "react"

import { GatsbyImage } from "gatsby-plugin-image"

const PluginImage = ({ data }) => {
const images = data.allMyRemoteFile.nodes
const [nodeIndex, updateNode] = React.useState(0)

return (
<div>
<h1>Image Testing</h1>

<div id="image-wrapper">
<GatsbyImage
image={data.allMyRemoteFile.nodes[nodeIndex].fixed}
alt="fixed"
backgroundColor="rebeccapurple"
/>
</div>

<button
id="click"
onClick={() =>
updateNode(nodeIndex === images.length - 1 ? 0 : nodeIndex + 1)
}
>
Change Image
</button>
</div>
)
}

export const pageQuery = graphql`
{
allMyRemoteFile {
nodes {
id
fixed: gatsbyImage(layout: FIXED, width: 100, placeholder: NONE)
}
}
}
`

export default PluginImage
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { graphql } from "gatsby"
import React from "react"

import { GatsbyImage } from "gatsby-plugin-image"

const PluginImage = ({ data }) => {
const [bg, updateBg] = React.useState("rebeccapurple")

return (
<div>
<h1>Image Testing</h1>

<div id="image-wrapper">
<GatsbyImage
image={data.allMyRemoteFile.nodes[0].fixed}
alt="fixed"
backgroundColor={bg}
/>
</div>

<button
id="click"
onClick={() =>
updateBg(bg === "rebeccapurple" ? "red" : "rebeccapurple")
}
>
Change Image
</button>
</div>
)
}

export const pageQuery = graphql`
{
allMyRemoteFile {
nodes {
id
fixed: gatsbyImage(layout: FIXED, width: 100, placeholder: NONE)
}
}
}
`

export default PluginImage
6 changes: 0 additions & 6 deletions packages/gatsby-plugin-image/gatsby-browser.js

This file was deleted.

7 changes: 3 additions & 4 deletions packages/gatsby-plugin-image/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
"build:gatsby-node": "tsc --jsx react --downlevelIteration true --skipLibCheck true --esModuleInterop true --outDir dist/ src/gatsby-node.ts src/babel-plugin-parse-static-images.ts src/resolver-utils.ts src/types.d.ts -d --declarationDir dist/src",
"build:gatsby-ssr": "microbundle -i src/gatsby-ssr.tsx -f cjs -o ./[name].js --no-pkg-main --jsx React.createElement --no-compress --external=common-tags,react --no-sourcemap",
"build:server": "microbundle -f cjs,es --jsx React.createElement --define SERVER=true",
"build:browser": "microbundle -i src/index.browser.ts -f cjs,modern,es --jsx React.createElement -o dist/gatsby-image.browser --define SERVER=false",
"build:browser": "microbundle -i src/index.browser.ts -f cjs,modern --jsx React.createElement -o dist/gatsby-image.browser --define SERVER=false",
"prepare": "yarn build",
"watch": "run-p watch:*",
"watch": "npm-run-all -s clean -p watch:*",
"watch:gatsby-node": "yarn build:gatsby-node --watch",
"watch:gatsby-ssr": "yarn build:gatsby-ssr watch",
"watch:server": "yarn build:server --no-compress watch",
Expand All @@ -27,8 +27,7 @@
"esmodule": "dist/gatsby-image.modern.js",
"browser": {
"./dist/gatsby-image.js": "./dist/gatsby-image.browser.js",
"./dist/gatsby-image.module.js": "./dist/gatsby-image.browser.module.js",
"./dist/gatsby-image.modern.js": "./dist/gatsby-image.browser.modern.js"
"./dist/gatsby-image.module.js": "./dist/gatsby-image.browser.modern.js"
},
"files": [
"dist/*",
Expand Down
Loading