-
Notifications
You must be signed in to change notification settings - Fork 89
chore: 🚙 type some helper files and constants #860
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,22 @@ | ||
const { | ||
posix: { join }, | ||
} = require('path') | ||
import { posix } from 'path' | ||
|
||
export const restoreCache = async ({ cache, publish }) => { | ||
const cacheDir = posix.join(publish, 'cache') | ||
|
||
exports.restoreCache = async ({ cache, publish }) => { | ||
const cacheDir = join(publish, 'cache') | ||
if (await cache.restore(cacheDir)) { | ||
console.log('Next.js cache restored.') | ||
} else { | ||
console.log('No Next.js cache to restore.') | ||
} | ||
} | ||
|
||
exports.saveCache = async ({ cache, publish }) => { | ||
const cacheDir = join(publish, 'cache') | ||
export const saveCache = async ({ cache, publish }) => { | ||
const cacheDir = posix.join(publish, 'cache') | ||
|
||
const buildManifest = join(publish, 'build-manifest.json') | ||
const buildManifest = posix.join(publish, 'build-manifest.json') | ||
if (await cache.save(cacheDir, { digests: [buildManifest] })) { | ||
console.log('Next.js cache saved.') | ||
} else { | ||
console.log('No Next.js cache to save.') | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,29 @@ | ||
/* eslint-disable max-lines */ | ||
|
||
const { yellowBright } = require('chalk') | ||
const { readJSON, existsSync } = require('fs-extra') | ||
const { outdent } = require('outdent') | ||
const { join, dirname, relative } = require('pathe') | ||
const slash = require('slash') | ||
import { NetlifyConfig } from '@netlify/build' | ||
import { yellowBright } from 'chalk' | ||
import { readJSON } from 'fs-extra' | ||
import { PrerenderManifest } from 'next/dist/build' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I just mean use the type import syntax to make it clear that you're only importing the type:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohhh TIL |
||
import { outdent } from 'outdent' | ||
import { join, dirname, relative } from 'pathe' | ||
import slash from 'slash' | ||
|
||
const defaultFailBuild = (message, { error }) => { | ||
import { HANDLER_FUNCTION_NAME, ODB_FUNCTION_NAME, HIDDEN_PATHS } from '../constants' | ||
|
||
import { RequiredServerFiles } from './requiredServerFilesType' | ||
|
||
const defaultFailBuild = (message: string, { error } ): never => { | ||
throw new Error(`${message}\n${error && error.stack}`) | ||
} | ||
|
||
const { HANDLER_FUNCTION_NAME, ODB_FUNCTION_NAME, HIDDEN_PATHS } = require('../constants') | ||
|
||
const ODB_FUNCTION_PATH = `/.netlify/builders/${ODB_FUNCTION_NAME}` | ||
const HANDLER_FUNCTION_PATH = `/.netlify/functions/${HANDLER_FUNCTION_NAME}` | ||
|
||
const CATCH_ALL_REGEX = /\/\[\.{3}(.*)](.json)?$/ | ||
const OPTIONAL_CATCH_ALL_REGEX = /\/\[{2}\.{3}(.*)]{2}(.json)?$/ | ||
const DYNAMIC_PARAMETER_REGEX = /\/\[(.*?)]/g | ||
|
||
const getNetlifyRoutes = (nextRoute) => { | ||
const getNetlifyRoutes = (nextRoute: string): Array<string> => { | ||
let netlifyRoutes = [nextRoute] | ||
|
||
// If the route is an optional catch-all route, we need to add a second | ||
|
@@ -54,8 +58,12 @@ const getNetlifyRoutes = (nextRoute) => { | |
return netlifyRoutes | ||
} | ||
|
||
exports.generateRedirects = async ({ netlifyConfig, basePath, i18n }) => { | ||
const { dynamicRoutes, routes: staticRoutes } = await readJSON( | ||
export const generateRedirects = async ({ netlifyConfig, basePath, i18n }: { | ||
netlifyConfig: NetlifyConfig, | ||
basePath: string, | ||
i18n | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can get this type as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oo thanks! Wasn't sure where it was coming from |
||
}) => { | ||
const { dynamicRoutes, routes: staticRoutes }: PrerenderManifest = await readJSON( | ||
join(netlifyConfig.build.publish, 'prerender-manifest.json'), | ||
) | ||
|
||
|
@@ -123,6 +131,8 @@ exports.generateRedirects = async ({ netlifyConfig, basePath, i18n }) => { | |
from: `${basePath}/*`, | ||
to: HANDLER_FUNCTION_PATH, | ||
status: 200, | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I left that in as I did the first pass to TS. I figured we'd want to get this in incrementally and worry too much about problematic missing types for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this type missing in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nah just one that would require a bit of refactoring of the function this ts-ignore refers to |
||
conditions: { Cookie: ['__prerender_bypass', '__next_preview_data'] }, | ||
force: true, | ||
}, | ||
|
@@ -158,14 +168,16 @@ exports.generateRedirects = async ({ netlifyConfig, basePath, i18n }) => { | |
} | ||
} | ||
|
||
exports.getNextConfig = async function getNextConfig({ publish, failBuild = defaultFailBuild }) { | ||
export const getNextConfig = async function getNextConfig({ publish, failBuild = defaultFailBuild }) { | ||
try { | ||
const { config, appDir, ignore } = await readJSON(join(publish, 'required-server-files.json')) | ||
const { config, appDir, ignore }: RequiredServerFiles = await readJSON(join(publish, 'required-server-files.json')) | ||
if (!config) { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, expects a second object and I just ignored it as I passed through :P |
||
return failBuild('Error loading your Next config') | ||
} | ||
return { ...config, appDir, ignore } | ||
} catch (error) { | ||
} catch (error: unknown) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is best practice to have errors as |
||
return failBuild('Error loading your Next config', { error }) | ||
} | ||
} | ||
|
@@ -180,7 +192,7 @@ const resolveModuleRoot = (moduleName) => { | |
|
||
const DEFAULT_EXCLUDED_MODULES = ['sharp', 'electron'] | ||
|
||
exports.configureHandlerFunctions = ({ netlifyConfig, publish, ignore = [] }) => { | ||
export const configureHandlerFunctions = ({ netlifyConfig, publish, ignore = [] }) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can these args be typed? |
||
/* eslint-disable no-underscore-dangle */ | ||
netlifyConfig.functions._ipx ||= {} | ||
netlifyConfig.functions._ipx.node_bundler = 'nft' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
/* eslint-disable eslint-comments/disable-enable-pair, @typescript-eslint/no-empty-interface, no-use-before-define */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, nice. Maybe we should contribute this upstream! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call! Although I'm sure the definition file is missing some types since it was just auto-generated based on the demo's |
||
/** | ||
* This was generated using @see {@link https://quicktype.io/}, and using the | ||
* demo's {@code demos/default/.next/required-server-files.json} file. I was | ||
* unable to find any types for this file so instead this was manually generated | ||
*/ | ||
|
||
export interface RequiredServerFiles { | ||
version?: number; | ||
config?: Config; | ||
appDir?: string; | ||
files?: string[]; | ||
ignore?: string[]; | ||
} | ||
|
||
export interface Env { | ||
} | ||
|
||
export interface Config { | ||
env?: Env; | ||
webpack?: null; | ||
webpackDevMiddleware?: null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ^ these will have to be manually updated, since it's clearly wrong |
||
eslint?: Eslint; | ||
typescript?: Typescript; | ||
distDir?: string; | ||
cleanDistDir?: boolean; | ||
assetPrefix?: string; | ||
configOrigin?: string; | ||
useFileSystemPublicRoutes?: boolean; | ||
generateEtags?: boolean; | ||
pageExtensions?: string[]; | ||
target?: string; | ||
poweredByHeader?: boolean; | ||
compress?: boolean; | ||
analyticsId?: string; | ||
images?: Images; | ||
devIndicators?: DevIndicators; | ||
onDemandEntries?: OnDemandEntries; | ||
amp?: Amp; | ||
basePath?: string; | ||
sassOptions?: Env; | ||
trailingSlash?: boolean; | ||
i18n?: I18N; | ||
productionBrowserSourceMaps?: boolean; | ||
optimizeFonts?: boolean; | ||
excludeDefaultMomentLocales?: boolean; | ||
serverRuntimeConfig?: Env; | ||
publicRuntimeConfig?: Env; | ||
reactStrictMode?: boolean; | ||
httpAgentOptions?: HTTPAgentOptions; | ||
outputFileTracing?: boolean; | ||
staticPageGenerationTimeout?: number; | ||
swcMinify?: boolean; | ||
experimental?: Experimental; | ||
future?: Future; | ||
configFileName?: string; | ||
} | ||
|
||
export interface Amp { | ||
canonicalBase?: string; | ||
} | ||
|
||
export interface DevIndicators { | ||
buildActivity?: boolean; | ||
buildActivityPosition?: string; | ||
} | ||
|
||
export interface Eslint { | ||
ignoreDuringBuilds?: boolean; | ||
} | ||
|
||
export interface Experimental { | ||
cpus?: number; | ||
sharedPool?: boolean; | ||
plugins?: boolean; | ||
profiling?: boolean; | ||
isrFlushToDisk?: boolean; | ||
workerThreads?: boolean; | ||
pageEnv?: boolean; | ||
optimizeImages?: boolean; | ||
optimizeCss?: boolean; | ||
scrollRestoration?: boolean; | ||
externalDir?: boolean; | ||
reactRoot?: boolean; | ||
disableOptimizedLoading?: boolean; | ||
gzipSize?: boolean; | ||
craCompat?: boolean; | ||
esmExternals?: boolean; | ||
isrMemoryCacheSize?: number; | ||
concurrentFeatures?: boolean; | ||
serverComponents?: boolean; | ||
fullySpecified?: boolean; | ||
outputFileTracingRoot?: string; | ||
outputStandalone?: boolean; | ||
} | ||
|
||
export interface Future { | ||
strictPostcssConfiguration?: boolean; | ||
} | ||
|
||
export interface HTTPAgentOptions { | ||
keepAlive?: boolean; | ||
} | ||
|
||
export interface I18N { | ||
defaultLocale?: string; | ||
locales?: string[]; | ||
} | ||
|
||
export interface Images { | ||
deviceSizes?: number[]; | ||
imageSizes?: number[]; | ||
path?: string; | ||
loader?: string; | ||
domains?: any[]; | ||
disableStaticImages?: boolean; | ||
minimumCacheTTL?: number; | ||
formats?: string[]; | ||
} | ||
|
||
export interface OnDemandEntries { | ||
maxInactiveAge?: number; | ||
pagesBufferLength?: number; | ||
} | ||
|
||
export interface Typescript { | ||
ignoreBuildErrors?: boolean; | ||
tsconfigPath?: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,50 +70,7 @@ exports.resolvePages = () => { | |
}" | ||
`; | ||
|
||
exports[`onBuild() generates static files manifest 1`] = ` | ||
Array [ | ||
"en/getStaticProps/1.html", | ||
"en/getStaticProps/1.json", | ||
"en/getStaticProps/2.html", | ||
"en/getStaticProps/2.json", | ||
"en/getStaticProps/env.html", | ||
"en/getStaticProps/env.json", | ||
"en/getStaticProps/static.html", | ||
"en/getStaticProps/static.json", | ||
"en/getStaticProps/withFallback/3.html", | ||
"en/getStaticProps/withFallback/3.json", | ||
"en/getStaticProps/withFallback/4.html", | ||
"en/getStaticProps/withFallback/4.json", | ||
"en/getStaticProps/withFallback/my/path/1.html", | ||
"en/getStaticProps/withFallback/my/path/1.json", | ||
"en/getStaticProps/withFallback/my/path/2.html", | ||
"en/getStaticProps/withFallback/my/path/2.json", | ||
"en/getStaticProps/withFallbackBlocking/3.html", | ||
"en/getStaticProps/withFallbackBlocking/3.json", | ||
"en/getStaticProps/withFallbackBlocking/4.html", | ||
"en/getStaticProps/withFallbackBlocking/4.json", | ||
"en/image.html", | ||
"en/previewTest.html", | ||
"en/previewTest.json", | ||
"en/static.html", | ||
"es/getStaticProps/env.html", | ||
"es/getStaticProps/env.json", | ||
"es/getStaticProps/static.html", | ||
"es/getStaticProps/static.json", | ||
"es/image.html", | ||
"es/previewTest.html", | ||
"es/previewTest.json", | ||
"es/static.html", | ||
"fr/getStaticProps/env.html", | ||
"fr/getStaticProps/env.json", | ||
"fr/getStaticProps/static.html", | ||
"fr/getStaticProps/static.json", | ||
"fr/image.html", | ||
"fr/previewTest.html", | ||
"fr/previewTest.json", | ||
"fr/static.html", | ||
] | ||
`; | ||
exports[`onBuild() generates static files manifest 1`] = `Array []`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You updated the snapshot after running |
||
|
||
exports[`onBuild() writes correct redirects to netlifyConfig 1`] = ` | ||
Array [ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I htink it's a good idea if we ensure that all exported functions have typed args and return values, even if they're just defined inline.