fix(types): fix @netlify/headers-parser types #6104

serhalp · 2025-02-25T21:26:55Z

Summary

fix `@netlify/headers-parser` types

These types were very weak, containing plentiful anys, both explicit and implicit, as well as some incorrect inferred types. This overhauls those types and fixes uncovered errors.

(This is needed to fix type errors in the CLI.)

simplify/fix tsconfig strict config

We should be opting into strict mode and only opting out of some flags incrementally while we fix errors. The current setup makes this difficult.

This PR:

flips the global strict flag on
removes values redundantly being set to the default
stops disabling flags that obscured no errors (or very few, which I then fixed)
moves a few flag disables to the specific packages that require them
explicitly configures strict flags for already rather strict packages (just edge-bundler, and now also headers-parser)

There's a lot more work to do here, but this is a start...

serhalp · 2025-02-25T21:27:45Z

packages/build/tsconfig.json

@@ -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. */


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

serhalp · 2025-02-25T21:28:17Z

packages/nock-udp/tsconfig.json

@@ -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. */


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

serhalp · 2025-02-25T21:28:33Z

packages/testing/tsconfig.json

@@ -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. */


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

serhalp · 2025-02-25T21:29:00Z

packages/zip-it-and-ship-it/src/config.ts

@@ -2,6 +2,8 @@ import { promises as fs } from 'fs'
 import { basename, extname, dirname, join } from 'path'

 import isPathInside from 'is-path-inside'
+// @ts-expect-error(serhalp) -- Remove once https://github.com/schnittstabil/merge-options/pull/28 is merged, or replace


This seemed worthwhile, as it allowed enabling noImplicitAny in zisi

serhalp · 2025-02-25T21:29:58Z

packages/zip-it-and-ship-it/tsconfig.json

-    "strict": true
+    "noImplicitAny": true,
+    "strictFunctionTypes": true,
+    "strictPropertyInitialization": true,
+    "useUnknownInCatchVariables": false,
+    "exactOptionalPropertyTypes": false,
+    "noImplicitReturns": false,
+    "noUncheckedIndexedAccess": false,
+    "noImplicitOverride": true,
+    "noPropertyAccessFromIndexSignature": false


The intent here was presumably to opt into strict mode, but the tsconfig files get merged on a field-by-field basis... so "strict": true here wasn't really doing anything, as the specific flags were all still disabled from the base config.

I reenabled what I could but had to leave a few disabled 😞

serhalp · 2025-02-25T21:33:23Z

packages/headers-parser/src/line_parser.ts


 import { pathExists } from 'path-exists'

 import { splitResults } from './results.js'
+import type { MinimalHeader } from './types.js'
+
+type RawHeader = { path: string } | { name: string; value: string }


Suggested change

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

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

serhalp · 2025-02-25T21:33:32Z

packages/headers-parser/src/all.ts

@@ -3,6 +3,8 @@ 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 }


Suggested change

export type { Header, MinimalHeader }

export type { Header, MinimalHeader }

serhalp · 2025-02-25T21:36:06Z

packages/headers-parser/src/normalize.ts

  if (!Array.isArray(headers)) {
    const error = new TypeError(`Headers must be an array not: ${headers}`)
-    return splitResults([error])
+    return splitResults<never>([error])


This looks funny but is correct: splitResults takes an array of T | Error and returns {values: T[]: errors: Error[]}; when given a literal array of type Error[] it can't infer T, so I explicitly pass in never as T.

Can you add this as an inline comment?

This requires the fixes in netlify/build#6104.

These types were very weak, containing plentiful `any`s, both explicit and implicit, as well as some incorrect inferred types.

We should be opting *into* strict mode and only opting *out* of some flags incrementally while we fix errors. This commit: - flips the global `strict` on - removes values being set to the default via the above - stops disabling flags that obscured no errors (or very few, which I then fixed) - moves a few flag disablings to the specific packages that require it - explictly configures strict flags for already rather strict packages

ndhoule · 2025-02-27T21:56:04Z

packages/headers-parser/src/all.ts

+  configHeaders: undefined | MinimalHeader[]
+  minimal: undefined | boolean


should these two be optional?

ndhoule · 2025-02-27T21:58:30Z

packages/headers-parser/src/all.ts

  const resultsArrays = await Promise.all(headersFiles.map(parseFileHeaders))
  return concatResults(resultsArrays)
 }

-const getConfigHeaders = async function (netlifyConfigPath) {
+const getConfigHeaders = async function (netlifyConfigPath?: undefined | string) {


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.)

ndhoule · 2025-02-27T22:00:24Z

packages/headers-parser/src/normalize.ts

  if (!Array.isArray(headers)) {
    const error = new TypeError(`Headers must be an array not: ${headers}`)
-    return splitResults([error])
+    return splitResults<never>([error])


Can you add this as an inline comment?

This requires the fixes in netlify/build#6104.

serhalp commented Feb 25, 2025

View reviewed changes

serhalp added a commit to netlify/cli that referenced this pull request Feb 27, 2025

types: fix misc. util types

45823ce

This requires the fixes in netlify/build#6104.

serhalp mentioned this pull request Feb 27, 2025

fix(types): fix several suppressed type errors in src/utils netlify/cli#7068

Draft

1 task

serhalp added 3 commits February 27, 2025 16:15

fix(types): fix @netlify/headers-parser types

698febd

These types were very weak, containing plentiful `any`s, both explicit and implicit, as well as some incorrect inferred types.

fix(types): mark netlify.toml [[headers]].values required

1fa08ca

serhalp force-pushed the serhalp/cpla-2534-fix-all-generated-ts-expect-errors branch from f53fa6b to 1fa08ca Compare February 27, 2025 21:15

serhalp marked this pull request as ready for review February 27, 2025 21:23

serhalp requested a review from a team as a code owner February 27, 2025 21:23

ndhoule reviewed Feb 27, 2025

View reviewed changes

ndhoule approved these changes Feb 27, 2025

View reviewed changes

serhalp added a commit to netlify/cli that referenced this pull request Feb 27, 2025

types: fix misc. util types

bde81c2

This requires the fixes in netlify/build#6104.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(types): fix @netlify/headers-parser types #6104

fix(types): fix @netlify/headers-parser types #6104

serhalp commented Feb 25, 2025

serhalp Feb 25, 2025

serhalp Feb 25, 2025

serhalp Feb 25, 2025

serhalp Feb 25, 2025

serhalp Feb 25, 2025 •

edited

Loading

serhalp Feb 25, 2025

serhalp Feb 25, 2025

serhalp Feb 25, 2025

serhalp Feb 25, 2025

ndhoule Feb 27, 2025

ndhoule Feb 27, 2025

ndhoule Feb 27, 2025

ndhoule Feb 27, 2025

	type RawHeader = { path: string } \| { name: string; value: string }
	type RawHeaderLine = { path: string } \| { name: string; value: string }

	export type { Header, MinimalHeader }

	export type { Header, MinimalHeader }

		configHeaders: undefined \| MinimalHeader[]
		minimal: undefined \| boolean

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

fix(types): fix @netlify/headers-parser types #6104

Are you sure you want to change the base?

fix(types): fix @netlify/headers-parser types #6104

Conversation

serhalp commented Feb 25, 2025

Summary

fix @netlify/headers-parser types

simplify/fix tsconfig strict config

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serhalp Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fix `@netlify/headers-parser` types

serhalp Feb 25, 2025 •

edited

Loading