Skip to content

Commit a3fa646

Browse files
wardpeetwardpeet
authored
feat(gatsby-sharp): create more resilient wrapper around sharp (gatsbyjs#34339)
* feat(gatsby-sharp): add gatsby-sharp as sharp abstraction * move safe-sharp to gatsby/sharp when available * fix yarn.lock and snapshots because of deps * only require sharp once + typescript fixes * fix fallback safe-sharp * update sharp * fix jest babel resolution * Apply suggestions from code review
1 parent d319983 commit a3fa646

File tree

17 files changed

+258
-71
lines changed

17 files changed

+258
-71
lines changed

‎jest-transformer.js

+7
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,11 @@ const babelJest = require(`babel-jest`)
22

33
module.exports = babelJest.default.createTransformer({
44
presets: [`babel-preset-gatsby-package`],
5+
babelrcRoots: [
6+
// Keep the root as a root
7+
`.`,
8+
9+
// Also consider monorepo packages "root" and load their .babelrc files.
10+
`./packages/*`,
11+
],
512
})

‎packages/gatsby-plugin-manifest/src/__tests__/gatsby-node.js

-5
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,6 @@ describe(`Test plugin manifest options`, () => {
106106
sharp.mockClear()
107107
})
108108

109-
// the require of gatsby-node performs the invoking
110-
it(`invokes sharp.simd for optimization`, () => {
111-
expect(sharp.simd).toHaveBeenCalledTimes(1)
112-
})
113-
114109
it(`correctly works with default parameters`, async () => {
115110
await onPostBootstrap(apiArgs, {
116111
name: `GatsbyJS`,

‎packages/gatsby-plugin-manifest/src/gatsby-node.js

+4-6
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,13 @@
11
import * as fs from "fs"
22
import * as path from "path"
3-
import sharp from "./safe-sharp"
3+
// TODO(v5): use gatsby/sharp
4+
import getSharpInstance from "./safe-sharp"
45
import { createContentDigest, slash } from "gatsby-core-utils"
56
import { defaultIcons, addDigestToPath, favicons } from "./common"
67
import { doesIconExist } from "./node-helpers"
78

89
import pluginOptionsSchema from "./pluginOptionsSchema"
910

10-
sharp.simd(true)
11-
12-
// force it to be 1 as we only resize one image
13-
sharp.concurrency(1)
14-
1511
async function generateIcon(icon, srcIcon) {
1612
const imgPath = path.join(`public`, icon.src)
1713

@@ -28,6 +24,7 @@ async function generateIcon(icon, srcIcon) {
2824
// Sharp accept density from 1 to 2400
2925
const density = Math.min(2400, Math.max(1, size))
3026

27+
const sharp = await getSharpInstance()
3128
return sharp(srcIcon, { density })
3229
.resize({
3330
width: size,
@@ -194,6 +191,7 @@ const makeManifest = async ({
194191
)
195192
}
196193

194+
const sharp = await getSharpInstance()
197195
const sharpIcon = sharp(icon)
198196

199197
const metadata = await sharpIcon.metadata()

‎packages/gatsby-plugin-manifest/src/safe-sharp.js

+11-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,17 @@ try {
157157
originalConsoleError(msg, ...args)
158158
handleMessage(msg)
159159
}
160-
sharp = require(`sharp`)
160+
try {
161+
sharp = require(`gatsby/sharp`)
162+
} catch (e) {
163+
sharp = () => {
164+
const sharp = require(`sharp`)
165+
sharp.simd()
166+
sharp.concurrency(1)
167+
168+
return Promise.resolve(sharp)
169+
}
170+
}
161171
} catch (e) {
162172
handleMessage(e.toString())
163173
throw e

‎packages/gatsby-remark-images-contentful/src/__tests__/index.js

+3
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,12 @@ jest.mock(`sharp`, () => {
3030
const pipeline = {
3131
metadata: metadataMock,
3232
}
33+
3334
return pipeline
3435
}
3536

37+
sharp.simd = jest.fn()
38+
sharp.concurrency = jest.fn()
3639
sharp.metadataMock = metadataMock
3740

3841
return sharp

‎packages/gatsby-remark-images-contentful/src/index.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const { selectAll } = require(`unist-util-select`)
2-
const sharp = require(`./safe-sharp`)
2+
// TODO(v5): use gatsby/sharp
3+
const getSharpInstance = require(`./safe-sharp`)
34
const axios = require(`axios`)
45
const _ = require(`lodash`)
56
const Promise = require(`bluebird`)
@@ -62,6 +63,7 @@ module.exports = async (
6263
if (cachedRawHTML) {
6364
return cachedRawHTML
6465
}
66+
const sharp = await getSharpInstance()
6567
const metaReader = sharp()
6668

6769
// @todo to increase reliablility, this should use the asset downloading function from gatsby-source-contentful
@@ -86,6 +88,7 @@ module.exports = async (
8688
try {
8789
metadata = await metaReader.metadata()
8890
} catch (error) {
91+
console.log(error)
8992
reporter.panic(
9093
`The image "${node.url}" (with alt text: "${node.alt}") doesn't appear to be a supported image format.`,
9194
error

‎packages/gatsby-remark-images-contentful/src/safe-sharp.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,18 @@ try {
157157
originalConsoleError(msg, ...args)
158158
handleMessage(msg)
159159
}
160-
sharp = require(`sharp`)
160+
try {
161+
sharp = require(`gatsby/sharp`)
162+
} catch (e) {
163+
sharp = () => {
164+
const sharp = require(`sharp`)
165+
166+
sharp.simd()
167+
sharp.concurrency(1)
168+
169+
return Promise.resolve(sharp)
170+
}
171+
}
161172
} catch (e) {
162173
handleMessage(e.toString())
163174
throw e

‎packages/gatsby-sharp/.babelrc.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
console.log('hi there');
2+
module.exports = {
3+
"presets": [["babel-preset-gatsby-package"]],
4+
"plugins": ["babel-plugin-replace-ts-export-assignment"]
5+
}

‎packages/gatsby-sharp/package.json

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
{
2+
"name": "gatsby-sharp",
3+
"version": "0.0.1-next.0",
4+
"sideEffects": false,
5+
"keywords": [
6+
"gatsby",
7+
"sharp"
8+
],
9+
"main": "dist/index.js",
10+
"source": "src/index.ts",
11+
"files": [
12+
"dist/*"
13+
],
14+
"types": "dist/index.d.ts",
15+
"homepage": "https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-sharp#readme",
16+
"dependencies": {
17+
"@types/sharp": "^0.29.5",
18+
"sharp": "^0.29.3"
19+
},
20+
"devDependencies": {
21+
"@babel/cli": "^7.15.5",
22+
"@babel/core": "^7.15.5",
23+
"babel-plugin-replace-ts-export-assignment": "^0.0.2",
24+
"cross-env": "^7.0.3"
25+
},
26+
"engines": {
27+
"node": ">=14.15.0"
28+
},
29+
"repository": {
30+
"type": "git",
31+
"url": "https://github.com/gatsbyjs/gatsby.git",
32+
"directory": "packages/gatsby-sharp"
33+
},
34+
"license": "MIT",
35+
"scripts": {
36+
"build": "babel src --out-file dist/index.js --ignore \"**/__tests__\" --extensions \".ts,.js\"",
37+
"typegen": "tsc --emitDeclarationOnly --declaration --declarationDir dist/",
38+
"prepare": "cross-env NODE_ENV=production npm-run-all -s build typegen",
39+
"watch": "babel -w src --out-file dist/index.js --ignore \"**/__tests__\" --extensions \".ts,.js\""
40+
}
41+
}

‎packages/gatsby-sharp/src/__tests__/index.ts

+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/** @jest-environment node */
2+
import { exec } from "child_process"
3+
4+
jest.mock(`child_process`, () => {
5+
return {
6+
exec: jest.fn(async (command, options, cb) => {
7+
setImmediate(() => {
8+
try {
9+
return cb(
10+
null,
11+
`> [email protected] install C:\\Users\\Ward\\projects\\gatsby\\gatsby\\node_modules\\sharp\n`,
12+
``
13+
)
14+
} catch (err) {
15+
return cb(true, ``, err.message)
16+
}
17+
})
18+
}),
19+
}
20+
})
21+
22+
function getSharpInstance(): typeof import("../index") {
23+
let getSharpInstance
24+
jest.isolateModules(() => {
25+
getSharpInstance = require(`../index`)
26+
})
27+
28+
return getSharpInstance()
29+
}
30+
31+
describe(`getSharpInstance`, () => {
32+
beforeEach(() => {
33+
exec.mockClear()
34+
})
35+
36+
// jest mocking is making this impossible to test
37+
it(`should give you the bare sharp module`, async () => {
38+
const sharpInstance = await getSharpInstance()
39+
40+
expect(exec).not.toHaveBeenCalled()
41+
expect(sharpInstance).toBeDefined()
42+
expect(sharpInstance.versions).toBeDefined()
43+
})
44+
45+
it(
46+
`should rebuild sharp when binaries not found for current arch`,
47+
async () => {
48+
expect.assertions(3)
49+
50+
let called = false
51+
jest.doMock(`sharp`, () => {
52+
if (!called) {
53+
called = true
54+
throw new Error(`sharp failed to load`)
55+
}
56+
57+
return jest.requireActual(`sharp`)
58+
})
59+
60+
try {
61+
const sharpInstance = await getSharpInstance()
62+
expect(sharpInstance).toBeDefined()
63+
expect(sharpInstance.versions).toBeDefined()
64+
} catch (err) {
65+
// ignore
66+
}
67+
68+
expect(exec).toHaveBeenCalledWith(
69+
`npm rebuild sharp`,
70+
expect.anything(),
71+
expect.anything()
72+
)
73+
},
74+
60 * 1000
75+
)
76+
})

‎packages/gatsby-sharp/src/index.ts

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
const { exec } = require(`child_process`)
2+
const { createRequire } = require(`module`)
3+
4+
let sharpInstance: typeof import("sharp")
5+
6+
export = async function getSharpInstance(): Promise<typeof import("sharp")> {
7+
try {
8+
return importSharp()
9+
} catch (err) {
10+
await rebuildSharp()
11+
12+
// Try importing again now we have rebuilt sharp
13+
return importSharp()
14+
}
15+
}
16+
17+
function importSharp(): typeof import("sharp") {
18+
if (!sharpInstance) {
19+
const cleanRequire = createRequire(__filename)
20+
const sharp = cleanRequire(`sharp`)
21+
22+
sharp.simd(true)
23+
// Concurrency is handled by gatsby
24+
sharp.concurrency(1)
25+
26+
sharpInstance = sharp
27+
}
28+
29+
return sharpInstance
30+
}
31+
32+
function rebuildSharp(): Promise<string> {
33+
return new Promise((resolve, reject) => {
34+
exec(
35+
`npm rebuild sharp`,
36+
{
37+
timeout: 60 * 1000,
38+
},
39+
(error, stdout, stderr) => {
40+
if (error) {
41+
if (error.killed) {
42+
console.log(`timeout reached`)
43+
}
44+
45+
return reject(stderr)
46+
}
47+
48+
return setImmediate(() => resolve(stdout))
49+
}
50+
)
51+
})
52+
}

‎packages/gatsby-sharp/tsconfig.json

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"extends": "../../tsconfig.json",
3+
"compilerOptions": {
4+
"target": "ES5",
5+
"module": "CommonJS"
6+
},
7+
"exclude": [
8+
"src/__tests__",
9+
"src/__mocks__",
10+
"dist",
11+
]
12+
}

‎packages/gatsby-source-contentful/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
"gatsby": "^4.0.0-next",
4747
"gatsby-plugin-image": "^2.0.0-next",
4848
"gatsby-plugin-sharp": "^4.0.0-next",
49-
"sharp": "^0.29.0"
49+
"sharp": "^0.29.3"
5050
},
5151
"repository": {
5252
"type": "git",

‎packages/gatsby/package.json

+4
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
"@types/reach__router": "^1.3.5",
171171
"@types/react-dom": "^17.0.9",
172172
"@types/semver": "^7.3.9",
173+
"@types/sharp": "^0.29.5",
173174
"@types/signal-exit": "^3.0.0",
174175
"@types/string-similarity": "^4.0.0",
175176
"@types/tmp": "^0.2.0",
@@ -188,6 +189,9 @@
188189
"zipkin-javascript-opentracing": "^3.0.0",
189190
"zipkin-transport-http": "^0.22.0"
190191
},
192+
"optionalDependencies": {
193+
"gatsby-sharp": "^0.0.1-next.0"
194+
},
191195
"engines": {
192196
"node": ">=14.15.0"
193197
},

‎packages/gatsby/sharp.d.ts

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import getSharpInstance from "gatsby-sharp"
2+
3+
export = getSharpInstance

‎packages/gatsby/sharp.js

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"use strict"
2+
3+
module.exports = require('gatsby-sharp');

0 commit comments

Comments
 (0)