cli icon indicating copy to clipboard operation
cli copied to clipboard

refactor: type global configuration file

Open ndhoule opened this issue 9 months ago • 1 comments

Follow-up to a comment thread in https://github.com/netlify/cli/pull/7199.

This changeset adds types for the global configuration file ($CONFIG_HOME/netlify/config.json).

This is mainly a step forward in documenting the configuration format. The type safety on it isn't ideal; e.g. you can try to read or write properties that don't exist in the config schema. That said, it's an improvement on the read side in the sense that any property you read is typed--even unknown keys default to undefined.

This comes with two potential breaking changes; I didn't find that they actually broke anything in practice, but they're worth looking out for in review:

  1. We now set defaults for some values we did not previously set defaults for, e.g. users[id: string].auth defaults to an object. This could only break if a code path assumes e.g. the presence of auth.github means the user is authenticated; however, the way the types are structured marks all properties on that object as undefined, so this is a difficult error to make now.
  2. We now validate that users[id: string].id is set. Failing to set it would make looking up user configuration impossible (we index into this object using the top-level userId field), so this prevents a class of mysterious logic errors.

Stronger typing caused some issues in tests that mock the global config store; broke the GlobalConfigStore's storage logic out into an adapter and implemented an in-memory adapter for use in tests. There are better ways to solve this problem--tests should be able to mock the global config without resorting to module mocking--but this will do for now.

I think the global config store interface could use some work: at this point I think the dot-prop style of setting and getting properties only adds complexity vs something like a Proxy-wrapped object. That said, this is a step forward without committing to a bigger change.

ndhoule avatar Apr 21 '25 17:04 ndhoule

📊 Benchmark results

Comparing with 8d84e80c736ead5e813927ae524668c016008564

  • Dependency count: 1,155 ⬇️ 0.09% decrease vs. 8d84e80c736ead5e813927ae524668c016008564
  • Package size: 278 MB ⬇️ 0.82% decrease vs. 8d84e80c736ead5e813927ae524668c016008564
  • Number of ts-expect-error directives: 459 ⬇️ 1.31% decrease vs. 8d84e80c736ead5e813927ae524668c016008564

github-actions[bot] avatar Apr 21 '25 17:04 github-actions[bot]