From e2651929d5c5eea59290b4e178ad1089e15cfd8c Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Thu, 30 Jul 2020 00:14:52 +1200 Subject: [PATCH 1/6] feat: Add DJB2a hash function for generating `uniqueKey` `uniqueKey` is a CSS classname friendly base36 string of a 32-bit hash generated from all the image variants `srcSet` values. DJB2a itself is the XOR(`^`) improved version of DJB2, a simple and well known hash function. It has been selected for lightweight footprint and decent performance(minimal overhead), but may suffer slightly compared to FNV1a-32 collision wise. It should be a fairly safe choice to go with, `styled-components` also uses this hash function for a similar purpose. Special care is required for hash functions to work correctly in JS due to it's number type not being a 32-bit native type. Bitwise operators in JS operate in 32-bits however, and `>>>` ensures a positive uint number is returned to match expected hash outputs. Used in future commits for CSS to target, as a short and unique identifier. Only present during SSR, removed once hydrated. --- packages/gatsby-image/src/index.js | 34 ++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/gatsby-image/src/index.js b/packages/gatsby-image/src/index.js index 237db60cb2861..d6c1a9de6988c 100644 --- a/packages/gatsby-image/src/index.js +++ b/packages/gatsby-image/src/index.js @@ -18,6 +18,23 @@ const logDeprecationNotice = (prop, replacement) => { } } +// DJB2a (XOR variant) is a simple hashing function, reduces input to 32-bits +const SEED = 5381 +function djb2a(str) { + let h = SEED + + for (let i = 0; i < str.length; i++) { + h = (h * 33) ^ str.charCodeAt(i) + } + + // Cast to 32-bit uint + return h >>> 0 +} + +// Converts string input to a 32-bit base36 string (0-9, a-z) +// '_' prefix to prevent invalid first chars for CSS class names +const getShortKey = input => `_` + djb2a(input).toString(36) + // Handle legacy props during their deprecation phase const convertProps = props => { let convertedProps = { ...props } @@ -365,6 +382,13 @@ class Image extends React.Component { this.placeholderRef = props.placeholderRef || React.createRef() this.handleImageLoaded = this.handleImageLoaded.bind(this) this.handleRef = this.handleRef.bind(this) + + // Supports Art Direction feature to correctly render image variants + const imageVariants = props.fluid || props.fixed + if (hasArtDirectionSupport(imageVariants)) { + this.uniqueKey = + !isBrowser && getShortKey(imageVariants.map(v => v.srcSet).join(``)) + } } componentDidMount() { @@ -494,10 +518,14 @@ class Image extends React.Component { ? imageVariants[0] : getCurrentSrcData(imageVariants) + const uniqueKey = this.uniqueKey + if (fluid) { return ( Date: Thu, 30 Jul 2020 13:21:31 +1200 Subject: [PATCH 2/6] feat: Embed `} + {imageVariants + .filter(v => v.media) + .map(v => ( + + ))} + + ) +} + function generateTracedSVGSources(imageVariants) { return imageVariants.map(({ src, media, tracedSVG }) => ( @@ -536,6 +562,13 @@ class Image extends React.Component { ref={this.handleRef} key={`fluid-${JSON.stringify(image.srcSet)}`} > + {uniqueKey && ( + + )} + {/* Preserve the aspect ratio. */} + {uniqueKey && ( + + )} + {/* Show a solid background color. */} {bgColor && ( Date: Thu, 17 Sep 2020 14:52:16 +1200 Subject: [PATCH 3/6] chore: Use hash method with `key` prop Use the new hash method for `key` prop as well. Original commit for this was lost due to rebase with the other half of the commit was transferred to the hydration fix PR this PR compliments. --- packages/gatsby-image/src/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/gatsby-image/src/index.js b/packages/gatsby-image/src/index.js index 113691161780b..72814325e641c 100644 --- a/packages/gatsby-image/src/index.js +++ b/packages/gatsby-image/src/index.js @@ -544,6 +544,7 @@ class Image extends React.Component { ? imageVariants[0] : getCurrentSrcData(imageVariants) + const activeVariant = getShortKey(image.srcSet) const uniqueKey = this.uniqueKey if (fluid) { @@ -560,7 +561,7 @@ class Image extends React.Component { ...style, }} ref={this.handleRef} - key={`fluid-${JSON.stringify(image.srcSet)}`} + key={activeVariant} > {uniqueKey && ( {uniqueKey && ( Date: Thu, 17 Sep 2020 16:35:34 +1200 Subject: [PATCH 4/6] chore: Extract PR to HOC Reviewer does not want to merge the feature into `gatsby-image`. Extracted to separate a file as a Higher-Order Component. Reverts changes to `index.js`. Loses key prop improvement with `activeVariant`. May not integrate as smoothly as-is due to changes in hydration fix rendering, may need to keep `uniqueKey` until hydrated. To use, fluid or fixed prop is **expected** to be provided. Import the HOC version `gatsby-image/art-direction` and use as you would `gatsby-image`. --- packages/gatsby-image/src/art-direction.js | 75 ++++++++++++++++++++ packages/gatsby-image/src/index.js | 79 ++-------------------- 2 files changed, 79 insertions(+), 75 deletions(-) create mode 100644 packages/gatsby-image/src/art-direction.js diff --git a/packages/gatsby-image/src/art-direction.js b/packages/gatsby-image/src/art-direction.js new file mode 100644 index 0000000000000..80ede5db719cb --- /dev/null +++ b/packages/gatsby-image/src/art-direction.js @@ -0,0 +1,75 @@ +import React from "react" +import GatsbyImage from "index" + +const isBrowser = typeof window !== `undefined` + +// DJB2a (XOR variant) is a simple hashing function, reduces input to 32-bits +// Same hashing algorithm as used by `styled-components`. +const SEED = 5381 +function djb2a(str) { + let hash = SEED + + for (let i = 0; i < str.length; i++) { + hash = (hash * 33) ^ str.charCodeAt(i) + } + + // Cast to 32-bit uint + return hash >>> 0 +} + +// Converts string input to a 32-bit base36 string (0-9, a-z) +// '_' prefix to prevent invalid first chars for CSS class names +const getShortKey = input => `_` + djb2a(input).toString(36) + +// Creates the `} + {imageVariants + .filter(v => v.media) + .map(v => ( + + ))} + + ) +} + +// Supports Art Direction feature with `} - {imageVariants - .filter(v => v.media) - .map(v => ( - - ))} - - ) -} - function generateTracedSVGSources(imageVariants) { return imageVariants.map(({ src, media, tracedSVG }) => ( @@ -408,13 +365,6 @@ class Image extends React.Component { this.placeholderRef = props.placeholderRef || React.createRef() this.handleImageLoaded = this.handleImageLoaded.bind(this) this.handleRef = this.handleRef.bind(this) - - // Supports Art Direction feature to correctly render image variants - const imageVariants = props.fluid || props.fixed - if (hasArtDirectionSupport(imageVariants)) { - this.uniqueKey = - !isBrowser && getShortKey(imageVariants.map(v => v.srcSet).join(``)) - } } componentDidMount() { @@ -544,15 +494,10 @@ class Image extends React.Component { ? imageVariants[0] : getCurrentSrcData(imageVariants) - const activeVariant = getShortKey(image.srcSet) - const uniqueKey = this.uniqueKey - if (fluid) { return ( - {uniqueKey && ( - - )} - {/* Preserve the aspect ratio. */} - {uniqueKey && ( - - )} - {/* Show a solid background color. */} {bgColor && ( Date: Thu, 17 Sep 2020 17:02:17 +1200 Subject: [PATCH 5/6] chore: Add `key` prop in `ResponsiveQueries` Alternative single ` ) ``` --- packages/gatsby-image/src/art-direction.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/gatsby-image/src/art-direction.js b/packages/gatsby-image/src/art-direction.js index 80ede5db719cb..097653ebaf58b 100644 --- a/packages/gatsby-image/src/art-direction.js +++ b/packages/gatsby-image/src/art-direction.js @@ -44,7 +44,9 @@ const ResponsiveQueries = ({ imageVariants, selectorClass }) => { {imageVariants .filter(v => v.media) .map(v => ( - + ))} ) From 0c0ae424fc12fba1b2e12a89d579ebcaddf077ce Mon Sep 17 00:00:00 2001 From: polarathene <5098581+polarathene@users.noreply.github.com> Date: Thu, 17 Sep 2020 19:18:45 +1200 Subject: [PATCH 6/6] fix: Properly target internal elements Added support for optional `maxWidth` and `maxHeight` for `fluid` type images. The internal elements (padding for fluid, bgcolor for fixed) are both lacking class names to target specifically but both are the 1st children. Due to review constraints they are being targeted with a pseudo-selector, if the internal structure changes this will of course break. Note that the `fixed` type image has width/height attributes as well that are hard-coded to the first image in SSR build. Not tested if that causes any problems, I think it might affect `object-fit: cover`, but only with `critical` images where these attr values are used until image request responds(or re-validates if cached). --- packages/gatsby-image/src/art-direction.js | 29 +++++++++++++++------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/gatsby-image/src/art-direction.js b/packages/gatsby-image/src/art-direction.js index 097653ebaf58b..a6950ced87545 100644 --- a/packages/gatsby-image/src/art-direction.js +++ b/packages/gatsby-image/src/art-direction.js @@ -25,14 +25,25 @@ const getShortKey = input => `_` + djb2a(input).toString(36) // `emotion` does similar with ` ))} @@ -60,7 +71,7 @@ const withArtDirection = props => { const imageVariants = props.fluid || props.fixed const uniqueKey = !isBrowser - ? getShortKey(imageVariants.map(v => v.srcSet).join(``)) + ? ` ` + getShortKey(imageVariants.map(v => v.srcSet).join(``)) : `` return ( @@ -69,7 +80,7 @@ const withArtDirection = props => { imageVariants={imageVariants} selectorClass={uniqueKey} /> - + ) }