Skip to content

feat: write granular info into skip file #128

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

Merged
merged 4 commits into from
May 18, 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
68 changes: 56 additions & 12 deletions src/__tests__/gatsby-node.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable max-lines-per-function */
jest.mock('../plugin-data', () => ({
__esModule: true,
default: jest.fn().mockReturnValue({
Expand All @@ -14,10 +15,12 @@ jest.mock('fs-extra', () => ({
existsSync: jest.fn(),
readFile: jest.fn(),
writeFile: jest.fn(),
writeJson: jest.fn(),
remove: jest.fn(),
}))

// Importing writeFile here gives us access to the mocked method to assert the correct content is written to the file within test cases
import { writeFile } from 'fs-extra'
import { writeFile, writeJson, remove } from 'fs-extra'
import { testPluginOptionsSchema } from 'gatsby-plugin-utils'

import { pluginOptionsSchema, onPostBuild } from '../gatsby-node'
Expand Down Expand Up @@ -67,12 +70,12 @@ describe(`gatsby-node.js`, () => {
})

describe('onPostBuild', () => {
let store = {}
const reporter = {
info: jest.fn(),
}
beforeEach(() => {
store = {

it('creates redirects for SSR and DSG pages in format Netlify expects', async () => {
const store = {
getState: jest.fn().mockReturnValue({
redirects: [],
pages: [
Expand All @@ -89,13 +92,6 @@ describe(`gatsby-node.js`, () => {
program: { directory: '' },
}),
}
})

afterEach(() => {
jest.resetAllMocks()
})

it('creates redirects for SSR and DSG pages in format Netlify expects', async () => {
const expectedValue = [
'',
'## Created with gatsby-plugin-netlify',
Expand All @@ -106,7 +102,55 @@ describe(`gatsby-node.js`, () => {
].join(`\n`)

await onPostBuild({ store, pathPrefix: '', reporter }, {})
expect(writeFile).toHaveBeenCalledWith('mock-file-path', expectedValue)
expect(writeFile).toHaveBeenLastCalledWith('mock-file-path', expectedValue)
})

it('creates skip file for unneeded function bundles', async () => {
const store = {
getState: jest.fn().mockReturnValue({
redirects: [],
pages: [
{
mode: 'DSG',
path: 'some/other/path',
},
],
functions: [],
program: { directory: '' },
}),
}
const expectedValue = {
API: false,
SSR: false,
DSG: true,
}

await onPostBuild({ store, pathPrefix: '', reporter }, {})
expect(writeJson).toHaveBeenLastCalledWith('.cache/.nf-skip-gatsby-functions', expectedValue)
})

it('removes skip file when all function bundles needed', async () => {
const store = {
getState: jest.fn().mockReturnValue({
redirects: [],
pages: [
{
mode: 'SSR',
path: 'some/path',
},
{
mode: 'DSG',
path: 'some/other/path',
},
],
functions: [true],
program: { directory: '' },
}),
}

await onPostBuild({ store, pathPrefix: '', reporter }, {})
expect(remove).toHaveBeenCalled()
})
})
})
/* eslint-enable max-lines-per-function */
19 changes: 12 additions & 7 deletions src/gatsby-node.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// https://www.netlify.com/docs/headers-and-basic-auth/
import { join } from 'path'

import { writeFile } from 'fs-extra'
import { writeJson, remove } from 'fs-extra'
import { generatePageDataPath } from 'gatsby-core-utils'
import WebpackAssetsManifest from 'webpack-assets-manifest'

Expand Down Expand Up @@ -72,15 +72,19 @@ export const onPostBuild = async ({ store, pathPrefix, reporter }: any, userPlug
let count = 0
const rewrites: any = []

let needsFunctions = functions.length !== 0
const neededFunctions = {
API: functions.length !== 0,
SSR: false,
DSG: false,
}

;[...pages.values()].forEach((page) => {
const { mode, matchPath, path } = page
const matchPathClean = matchPath && matchPath.replace(/\*.*/, '*')
const matchPathIsNotPath = matchPath && matchPath !== path

if (mode === `SSR` || mode === `DSG`) {
needsFunctions = true
neededFunctions[mode] = true
const fromPath = matchPathClean ?? path
const toPath = mode === `SSR` ? `/.netlify/functions/__ssr` : `/.netlify/functions/__dsg`
count++
Expand All @@ -103,12 +107,13 @@ export const onPostBuild = async ({ store, pathPrefix, reporter }: any, userPlug
})
reporter.info(`[gatsby-plugin-netlify] Created ${count} SSR/DSG redirect${count === 1 ? `` : `s`}...`)

if (!needsFunctions) {
reporter.info(`[gatsby-plugin-netlify] No Netlify functions needed. Skipping...`)
await writeFile(join(program.directory, `.cache`, `.nf-skip-gatsby-functions`), ``)
}
const skipFilePath = join(program.directory, `.cache`, `.nf-skip-gatsby-functions`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little confusing that the skip file sets the value "true" when it shouldn't be skipped, but that's probably fine for an internal API, and I can see why it's needed for backwards-compat

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I went round in circles with this one for a while! As you mentioned, for backwards-compat I ending up settling on this approach because I didn't want to rename the skip file and it was confusing to negate the values in netlify-plugin-gatsby when checking if a function should be bundled.

const generateSkipFile = Object.values(neededFunctions).includes(false)
? writeJson(skipFilePath, neededFunctions)
: remove(skipFilePath)

await Promise.all([
generateSkipFile,
buildHeadersProgram(pluginData, pluginOptions, reporter),
createRedirects(pluginData, redirects, rewrites),
])
Expand Down