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(types)!: fix @netlify/headers-parser types #6104

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion packages/build-info/src/settings/netlify-toml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ export type RequestHeaders = {
*/
export type Headers = {
for: For
values?: Values
values: Values
}
/**
* Define the actual headers.
Expand Down
3 changes: 2 additions & 1 deletion packages/build/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"outDir": "lib" /* Specify an output folder for all emitted files. */
"outDir": "lib" /* Specify an output folder for all emitted files. */,
"strictBindCallApply": false /* Check that the arguments for 'bind', 'call', and 'apply' methods match the original function. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this down to this specific package so I could enable it in the base config

},
"include": ["src/**/*.js", "src/**/*.ts"],
"exclude": ["tests/**"]
Expand Down
8 changes: 1 addition & 7 deletions packages/config/src/headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,11 @@ export const getHeadersPath = function ({ build: { publish } }) {
const HEADERS_FILENAME = '_headers'

// Add `config.headers`
export const addHeaders = async function ({
config: { headers: configHeaders, ...config },
headersPath,
logs,
featureFlags,
}) {
export const addHeaders = async function ({ config: { headers: configHeaders, ...config }, headersPath, logs }) {
const { headers, errors } = await parseAllHeaders({
headersFiles: [headersPath],
configHeaders,
minimal: true,
featureFlags,
})
warnHeadersParsing(logs, errors)
warnHeadersCaseSensitivity(logs, headers)
Expand Down
2 changes: 1 addition & 1 deletion packages/config/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ const getFullConfig = async function ({
base: baseA,
} = await resolveFiles({ packagePath, config: configA, repositoryRoot, base, baseRelDir })
const headersPath = getHeadersPath(configB)
const configC = await addHeaders({ config: configB, headersPath, logs, featureFlags })
const configC = await addHeaders({ config: configB, headersPath, logs })
const redirectsPath = getRedirectsPath(configC)
const configD = await addRedirects({ config: configC, redirectsPath, logs, featureFlags })
return { configPath, config: configD, buildDir, base: baseA, redirectsPath, headersPath }
Expand Down
2 changes: 1 addition & 1 deletion packages/config/src/mutations/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const updateConfig = async function (
const inlineConfig = applyMutations({}, configMutations)
const normalizedInlineConfig = ensureConfigPriority(inlineConfig, context, branch)
const updatedConfig = await mergeWithConfig(normalizedInlineConfig, configPath)
const configWithHeaders = await addHeaders({ config: updatedConfig, headersPath, logs, featureFlags })
const configWithHeaders = await addHeaders({ config: updatedConfig, headersPath, logs })
const finalConfig = await addRedirects({ config: configWithHeaders, redirectsPath, logs, featureFlags })
const simplifiedConfig = simplifyConfig(finalConfig)

Expand Down
14 changes: 11 additions & 3 deletions packages/headers-parser/src/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,22 @@ import { mergeHeaders } from './merge.js'
import { parseConfigHeaders } from './netlify_config_parser.js'
import { normalizeHeaders } from './normalize.js'
import { splitResults, concatResults } from './results.js'
import type { Header, MinimalHeader } from './types.js'

export type { Header, MinimalHeader }

// Parse all headers from `netlify.toml` and `_headers` file, then normalize
// and validate those.
export const parseAllHeaders = async function ({
headersFiles = [],
netlifyConfigPath,
configHeaders = [],
minimal = false,
minimal,
}: {
headersFiles: undefined | string[]
netlifyConfigPath?: undefined | string
configHeaders: undefined | MinimalHeader[]
minimal: boolean
}) {
const [
{ headers: fileHeaders, errors: fileParseErrors },
Expand All @@ -37,12 +45,12 @@ export const parseAllHeaders = async function ({
return { headers, errors }
}

const getFileHeaders = async function (headersFiles) {
const getFileHeaders = async function (headersFiles: string[]) {
const resultsArrays = await Promise.all(headersFiles.map(parseFileHeaders))
return concatResults(resultsArrays)
}

const getConfigHeaders = async function (netlifyConfigPath) {
const getConfigHeaders = async function (netlifyConfigPath?: undefined | string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const getConfigHeaders = async function (netlifyConfigPath?: undefined | string) {
const getConfigHeaders = async function (netlifyConfigPath?: string) {

undefined isn't required in optional parameters, even with exactOptionalPropertyTypes enabled. (I see this throughout the PR, but I'll just leave a single comment here.)

if (netlifyConfigPath === undefined) {
return splitResults([])
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import escapeStringRegExp from 'escape-string-regexp'

// Retrieve `forRegExp` which is a `RegExp` used to match the `for` path
export const getForRegExp = function (forPath) {
export const getForRegExp = function (forPath: string): RegExp {
const pattern = forPath.split('/').map(trimString).filter(Boolean).map(getPartRegExp).join('/')
return new RegExp(`^/${pattern}/?$`, 'iu')
}

const trimString = function (part) {
const trimString = function (part: string): string {
return part.trimEnd()
}

const getPartRegExp = function (part) {
const getPartRegExp = function (part: string): string {
// Placeholder like `/segment/:placeholder/test`
// Matches everything up to a /
if (part.startsWith(':')) {
Expand Down
2 changes: 1 addition & 1 deletion packages/headers-parser/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export { parseAllHeaders } from './all.js'
export { parseAllHeaders, type Header, type MinimalHeader } from './all.js'
46 changes: 32 additions & 14 deletions packages/headers-parser/src/line_parser.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
import { promises as fs } from 'fs'
import fs from 'fs/promises'

import { pathExists } from 'path-exists'

import { splitResults } from './results.js'
import type { MinimalHeader } from './types.js'

type RawHeaderFileLine = { path: string } | { name: string; value: string }

export interface ParseHeadersResult {
headers: MinimalHeader[]
errors: Error[]
}

// Parse `_headers` file to an array of objects following the same syntax as
// the `headers` property in `netlify.toml`
export const parseFileHeaders = async function (headersFile: string) {
export const parseFileHeaders = async function (headersFile: string): Promise<ParseHeadersResult> {
const results = await parseHeaders(headersFile)
const { headers, errors: parseErrors } = splitResults(results)
const { headers: reducedHeaders, errors: reducedErrors } = headers.reduce(reduceLine, { headers: [], errors: [] })
const errors = [...parseErrors, ...reducedErrors]
return { headers: reducedHeaders, errors }
}

const parseHeaders = async function (headersFile: string) {
const parseHeaders = async function (headersFile: string): Promise<Array<Error | RawHeaderFileLine>> {
if (!(await pathExists(headersFile))) {
return []
}
Expand All @@ -23,7 +31,12 @@ const parseHeaders = async function (headersFile: string) {
if (typeof text !== 'string') {
return [text]
}
return text.split('\n').map(normalizeLine).filter(hasHeader).map(parseLine).filter(Boolean)
return text
.split('\n')
.map(normalizeLine)
.filter(hasHeader)
.map(parseLine)
.filter((line): line is RawHeaderFileLine => line != null)
}

const readHeadersFile = async function (headersFile: string) {
Expand All @@ -38,22 +51,22 @@ const normalizeLine = function (line: string, index: number) {
return { line: line.trim(), index }
}

const hasHeader = function ({ line }) {
const hasHeader = function ({ line }: { line: string }) {
return line !== '' && !line.startsWith('#')
}

const parseLine = function ({ line, index }) {
const parseLine = function ({ line, index }: { line: string; index: number }) {
try {
return parseHeaderLine(line)
} catch (error) {
return new Error(`Could not parse header line ${index + 1}:
${line}
${error.message}`)
${error instanceof Error ? error.message : error?.toString()}`)
}
}

// Parse a single header line
const parseHeaderLine = function (line: string) {
const parseHeaderLine = function (line: string): undefined | RawHeaderFileLine {
if (isPathLine(line)) {
return { path: line }
}
Expand All @@ -63,7 +76,7 @@ const parseHeaderLine = function (line: string) {
}

const [rawName, ...rawValue] = line.split(HEADER_SEPARATOR)
const name = rawName.trim()
const name = rawName?.trim() ?? ''

if (name === '') {
throw new Error(`Missing header name`)
Expand All @@ -83,18 +96,23 @@ const isPathLine = function (line: string) {

const HEADER_SEPARATOR = ':'

const reduceLine = function ({ headers, errors }, { path, name, value }) {
if (path !== undefined) {
const reduceLine = function (
{ headers, errors }: ParseHeadersResult,
parsedHeader: RawHeaderFileLine,
): ParseHeadersResult {
if ('path' in parsedHeader) {
const { path } = parsedHeader
return { headers: [...headers, { for: path, values: {} }], errors }
}

if (headers.length === 0) {
const { name, value } = parsedHeader
const previousHeaders = headers.slice(0, -1)
const currentHeader = headers[headers.length - 1]
if (headers.length === 0 || currentHeader == null) {
const error = new Error(`Path should come before header "${name}"`)
return { headers, errors: [...errors, error] }
}

const previousHeaders = headers.slice(0, -1)
const currentHeader = headers[headers.length - 1]
const { values } = currentHeader
const newValue = values[name] === undefined ? value : `${values[name]}, ${value}`
const newHeaders = [...previousHeaders, { ...currentHeader, values: { ...values, [name]: newValue } }]
Expand Down
19 changes: 9 additions & 10 deletions packages/headers-parser/src/merge.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import stringify from 'fast-safe-stringify'

import { splitResults } from './results.js'
import type { Header } from './types.js'
import type { Header, MinimalHeader } from './types.js'

// Merge headers from `_headers` with the ones from `netlify.toml`.
// When:
Expand All @@ -21,8 +21,8 @@ export const mergeHeaders = function ({
fileHeaders,
configHeaders,
}: {
fileHeaders: (Error | Header)[]
configHeaders: (Error | Header)[]
fileHeaders: MinimalHeader[] | Header[]
configHeaders: MinimalHeader[] | Header[]
}) {
const results = [...fileHeaders, ...configHeaders]
const { headers, errors } = splitResults(results)
Expand All @@ -35,23 +35,22 @@ export const mergeHeaders = function ({
// `netlifyConfig.headers` is modified by plugins.
// The latest duplicate value is the one kept, hence why we need to iterate the
// array backwards and reverse it at the end
const removeDuplicates = function (headers: Header[]) {
const removeDuplicates = function (headers: MinimalHeader[] | Header[]) {
const uniqueHeaders = new Set()
const result: Header[] = []
for (let i = headers.length - 1; i >= 0; i--) {
const h = headers[i]
const key = generateHeaderKey(h)
const result: (MinimalHeader | Header)[] = []
for (const header of [...headers].reverse()) {
const key = generateHeaderKey(header)
if (uniqueHeaders.has(key)) continue
uniqueHeaders.add(key)
result.push(h)
result.push(header)
}
return result.reverse()
}

// We generate a unique header key based on JSON stringify. However, because some
// properties can be regexes, we need to replace those by their toString representation
// given the default will be and empty object
const generateHeaderKey = function (header: Header) {
const generateHeaderKey = function (header: MinimalHeader | Header): string {
return stringify.default.stableStringify(header, (_, value) => {
if (value instanceof RegExp) return value.toString()
return value
Expand Down
4 changes: 3 additions & 1 deletion packages/headers-parser/src/netlify_config_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { parse as loadToml } from '@iarna/toml'
import { pathExists } from 'path-exists'

import { splitResults } from './results.js'
import type { MinimalHeader } from './types.js'

// Parse `headers` field in "netlify.toml" to an array of objects.
// This field is already an array of objects, so it only validates and
Expand All @@ -27,7 +28,8 @@ const parseConfig = async function (configPath: string) {
if (!Array.isArray(headers)) {
throw new TypeError(`"headers" must be an array`)
}
return headers
// TODO(serhalp) Validate shape instead of assuming and asserting type
return headers as MinimalHeader[]
} catch (error) {
return [new Error(`Could not parse configuration file: ${error}`)]
}
Expand Down
Loading
Loading