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-sharp): don't write to same temporary file multiple times at a same time #12927

Merged
merged 2 commits into from
Apr 9, 2019
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
217 changes: 217 additions & 0 deletions packages/gatsby-plugin-sharp/src/__tests__/trace-svg.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
jest.mock(`sharp`, () => {
const sharp = path => {
const pipeline = {
rotate: () => pipeline,
resize: () => pipeline,
png: () => pipeline,
jpeg: () => pipeline,
toFile: (_, cb) => cb(),
}
return pipeline
}

sharp.simd = () => {}

return sharp
})

jest.mock(`potrace`, () => {
return {
trace: (_, _2, cb) => cb(null, ``),
Potrace: {
TURNPOLICY_MAJORITY: `wat`,
},
}
})

const path = require(`path`)

const traceSVGHelpers = require(`../trace-svg`)

const notMemoizedtraceSVG = jest.spyOn(traceSVGHelpers, `notMemoizedtraceSVG`)
const notMemoizedPrepareTraceSVGInputFile = jest.spyOn(
traceSVGHelpers,
`notMemoizedPrepareTraceSVGInputFile`
)
// note that we started spying on not memoized functions first
// now we recreate memoized functions that will use function we just started
// spying on
traceSVGHelpers.createMemoizedFunctions()
const memoizedTraceSVG = jest.spyOn(traceSVGHelpers, `memoizedTraceSVG`)
const memoizedPrepareTraceSVGInputFile = jest.spyOn(
traceSVGHelpers,
`memoizedPrepareTraceSVGInputFile`
)

const { traceSVG } = require(`../`)

function getFileObject(absolutePath, name = path.parse(absolutePath).name) {
return {
id: `${absolutePath} absPath of file`,
name: name,
absolutePath,
extension: `png`,
internal: {
contentDigest: `1234`,
},
}
}

describe(`traceSVG memoization`, () => {
const file = getFileObject(path.join(__dirname, `images/test.png`))
const copyOfFile = getFileObject(path.join(__dirname, `images/test-copy.png`))
const differentFile = getFileObject(
path.join(__dirname, `images/different.png`)
)
differentFile.internal.contentDigest = `4321`

beforeEach(() => {
traceSVGHelpers.clearMemoizeCaches()
memoizedTraceSVG.mockClear()
notMemoizedtraceSVG.mockClear()
memoizedPrepareTraceSVGInputFile.mockClear()
notMemoizedPrepareTraceSVGInputFile.mockClear()
})

it(`Baseline`, async () => {
await traceSVG({
file,
})

expect(memoizedTraceSVG).toBeCalledTimes(1)
expect(notMemoizedtraceSVG).toBeCalledTimes(1)
expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(1)
expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1)
})

it(`Is memoizing results for same args`, async () => {
await traceSVG({
file,
})

await traceSVG({
file,
})

expect(memoizedTraceSVG).toBeCalledTimes(2)
expect(notMemoizedtraceSVG).toBeCalledTimes(1)
expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(1)
expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1)
})

it(`Is calling functions with same input file when params change`, async () => {
await traceSVG({
file,
args: {
color: `red`,
},
fileArgs: {
width: 400,
},
})
await traceSVG({
file,
args: {
color: `blue`,
},
fileArgs: {
width: 400,
},
})
await traceSVG({
file,
args: {
color: `red`,
},
fileArgs: {
width: 200,
},
})
await traceSVG({
file,
args: {
color: `blue`,
},
fileArgs: {
width: 200,
},
})
await traceSVG({
file: differentFile,
args: {
color: `red`,
},
fileArgs: {
width: 400,
},
})

expect(memoizedTraceSVG).toBeCalledTimes(5)
expect(notMemoizedtraceSVG).toBeCalledTimes(5)
expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(5)
// trace svg should be actually created just 3 times
// because it's affected just by `fileArgs`, and not `args`
// this makes sure we don't try to write to same input file multiple times
expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(3)
expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith(
1,
expect.objectContaining({
file,
options: expect.objectContaining({
width: 400,
}),
})
)
expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith(
2,
expect.objectContaining({
file,
options: expect.objectContaining({
width: 200,
}),
})
)
expect(notMemoizedPrepareTraceSVGInputFile).toHaveBeenNthCalledWith(
3,
expect.objectContaining({
file: differentFile,
options: expect.objectContaining({
width: 400,
}),
})
)

const usedTmpFilePaths = notMemoizedPrepareTraceSVGInputFile.mock.calls.map(
args => args[0].tmpFilePath
)

// tmpFilePath was always unique
expect(usedTmpFilePaths.length).toBe(new Set(usedTmpFilePaths).size)
})

it(`Use memoized results for file copies`, async () => {
await traceSVG({
file,
args: {
color: `red`,
},
fileArgs: {
width: 400,
},
})
await traceSVG({
file: copyOfFile,
args: {
color: `red`,
},
fileArgs: {
width: 400,
},
})

expect(memoizedTraceSVG).toBeCalledTimes(2)
expect(notMemoizedtraceSVG).toBeCalledTimes(1)
expect(memoizedPrepareTraceSVGInputFile).toBeCalledTimes(1)
expect(notMemoizedPrepareTraceSVGInputFile).toBeCalledTimes(1)
})
})
4 changes: 3 additions & 1 deletion packages/gatsby-plugin-sharp/src/gatsby-node.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
const {
setBoundActionCreators,
setPluginOptions,
// queue: jobQueue,
// reportError,
} = require(`./index`)

const { setPluginOptions } = require(`./plugin-options`)

// const { scheduleJob } = require(`./scheduler`)
// let normalizedOptions = {}

Expand Down
Loading