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

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

Summary

Fix poor/incorrect 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.)

fix(@netlify/headers-parser)!: mark `parseAllHeaders` option `minimal` as required

All callers already pass in a non-nil boolean, so this is only technically breaking.

fix(@netlify/config)!: mark `NetlifyTOML.headers[].values` as required

This was just a mistake, but it is technically a breaking change.

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 😞

packages/headers-parser/src/line_parser.ts

packages/headers-parser/src/all.ts

packages/headers-parser/src/normalize.ts

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

packages/headers-parser/src/all.ts

packages/headers-parser/src/normalize.ts

This requires the fixes in netlify/build#6104.

github-actions · 2025-02-28T15:33:23Z

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

All callers already pass in a non-nil `boolean`, so this is only technically breaking.

github-actions · 2025-02-28T16:27:41Z

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

serhalp · 2025-02-28T16:30:37Z

packages/headers-parser/tests/helpers/main.ts

-  ...input
+  minimal,
 }: {
-  headersFiles?: undefined | string[]
-  netlifyConfigPath?: undefined | string
-  configHeaders?: undefined | MinimalHeader[]
-  minimal?: undefined | boolean
+  headersFiles: undefined | string[]
+  netlifyConfigPath: undefined | string
+  configHeaders: undefined | MinimalHeader[]
+  minimal: boolean
 }) {
  return await parseAllHeaders({
    ...(headersFiles && { headersFiles: headersFiles.map(addFileFixtureDir) }),
    ...(netlifyConfigPath && { netlifyConfigPath: addConfigFixtureDir(netlifyConfigPath) }),
    configHeaders,
-    // Default `minimal` to `true` but still allows passing `undefined` to
-    // test the default value of that option
-    minimal: 'minimal' in input ? input.minimal : true,
+    minimal,


This change is the reason why all the test files had to change. And this change is because of the change to parseAllHeaders.

(Yes, parseHeaders calls parseAllHeaders and not the other way around 🙃)

github-actions · 2025-02-28T16:37:19Z

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

This requires the fixes in netlify/build#6104.

serhalp commented Feb 25, 2025

View reviewed changes

packages/headers-parser/src/line_parser.ts Outdated Show resolved Hide resolved

serhalp commented Feb 25, 2025

View reviewed changes

packages/headers-parser/src/all.ts Show resolved Hide resolved

serhalp commented Feb 25, 2025

View reviewed changes

packages/headers-parser/src/normalize.ts Show resolved Hide resolved

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

packages/headers-parser/src/all.ts Outdated Show resolved Hide resolved

ndhoule previously approved these changes Feb 27, 2025

View reviewed changes

packages/headers-parser/src/all.ts Outdated Show resolved Hide resolved

packages/headers-parser/src/normalize.ts Show resolved Hide resolved

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.

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

types: fix misc. util types

727e837

This requires the fixes in netlify/build#6104.

serhalp added 3 commits February 28, 2025 10:32

style: add empty line after imports

6618e0a

refactor: improve variable name

58dc8cc

docs: add inline comment explain funky type

27e6287

serhalp dismissed ndhoule’s stale review via 41e53b7 February 28, 2025 15:32

github-actions bot added the Adds or modifies js files label Feb 28, 2025

serhalp changed the title ~~fix(types): fix @netlify/headers-parser types~~ fix(types)!: fix @netlify/headers-parser types Feb 28, 2025

serhalp added 3 commits February 28, 2025 10:39

refactor: remove incorrect extraneous property from type

fc3b1aa

fix(parseAllHeaders)!: mark minimal as required

62d0ded

All callers already pass in a non-nil `boolean`, so this is only technically breaking.

fix(types)!: increase strictness of optional types

e339cc6

serhalp force-pushed the serhalp/cpla-2534-fix-all-generated-ts-expect-errors branch from 41e53b7 to e339cc6 Compare February 28, 2025 16:27

serhalp commented Feb 28, 2025

View reviewed changes

serhalp added 2 commits February 28, 2025 11:36

fix(types): add missing PollingStrategy.name type

d09d2a4

feat(types): export Category, PollingStrategy

1d2bf01

serhalp requested a review from ndhoule February 28, 2025 16:37

ndhoule approved these changes Feb 28, 2025

View reviewed changes

serhalp merged commit bc5e35a into main Feb 28, 2025
34 checks passed

serhalp deleted the serhalp/cpla-2534-fix-all-generated-ts-expect-errors branch February 28, 2025 19:01

netlify-circleci mentioned this pull request Feb 28, 2025

chore: release main #6107

Open

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

types: fix misc. util types

41c5717

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 •

edited

Loading

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

github-actions bot commented Feb 28, 2025

github-actions bot commented Feb 28, 2025

serhalp Feb 28, 2025

github-actions bot commented Feb 28, 2025

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

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

Conversation

serhalp commented Feb 25, 2025 • edited Loading

Summary

Fix poor/incorrect types

fix(@netlify/headers-parser)!: mark parseAllHeaders option minimal as required

fix(@netlify/config)!: mark NetlifyTOML.headers[].values as required

Simplify/fix tsconfig strict config

serhalp Feb 25, 2025

Choose a reason for hiding this comment

serhalp Feb 25, 2025

Choose a reason for hiding this comment

serhalp Feb 25, 2025

Choose a reason for hiding this comment

serhalp Feb 25, 2025

Choose a reason for hiding this comment

serhalp Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

serhalp Feb 25, 2025

Choose a reason for hiding this comment

github-actions bot commented Feb 28, 2025

github-actions bot commented Feb 28, 2025

serhalp Feb 28, 2025

Choose a reason for hiding this comment

github-actions bot commented Feb 28, 2025

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

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

serhalp commented Feb 25, 2025 •

edited

Loading

fix(@netlify/headers-parser)!: mark `parseAllHeaders` option `minimal` as required

fix(@netlify/config)!: mark `NetlifyTOML.headers[].values` as required

serhalp Feb 25, 2025 •

edited

Loading