typescript-go icon indicating copy to clipboard operation
typescript-go copied to clipboard

Revamp user preferences serialization/deserialization

Open jakebailey opened this issue 2 weeks ago • 1 comments

In working on improving the fourslash testing, I wanted to make user preferences serializable, such that we can directly go from JSON to prefs and back, and therefore also test out the "actual" JSON encoding.

Making that work sort of snowballed into a total refactor, which eliminates the error-prone switch case for decoding, replacing it with struct tags and reflect.

Prefs now look like this:

type CodeLensUserPreferences struct {
	ReferencesCodeLensEnabled                     bool `raw:"referencesCodeLensEnabled" config:"referencesCodeLens.enabled"`
	ImplementationsCodeLensEnabled                bool `raw:"implementationsCodeLensEnabled" config:"implementationsCodeLens.enabled"`
	ReferencesCodeLensShowOnAllFunctions          bool `raw:"referencesCodeLensShowOnAllFunctions" config:"referencesCodeLens.showOnAllFunctions"`
	ImplementationsCodeLensShowOnInterfaceMethods bool `raw:"implementationsCodeLensShowOnInterfaceMethods" config:"implementationsCodeLens.showOnInterfaceMethods"`
	ImplementationsCodeLensShowOnAllClassMethods  bool `raw:"implementationsCodeLensShowOnAllClassMethods" config:"implementationsCodeLens.showOnAllClassMethods"`
}

A struct tag gives the name for the JSON propery (used by Strada), and also the name over LSP. https://github.com/microsoft/vscode/blob/main/extensions/typescript-language-features/package.json.

All prefs can still be set by the unstable object (which happens first, based on the raw name).

Structs can be nested, e.g.:

type UserPreferences struct {
	// ...
	ModuleSpecifier ModuleSpecifierUserPreferences
	// ...
}

type ModuleSpecifierUserPreferences struct {
	ImportModuleSpecifierPreference modulespecifiers.ImportModuleSpecifierPreference `raw:"importModuleSpecifierPreference" config:"preferences.importModuleSpecifier"` // !!!
	// Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js"
	ImportModuleSpecifierEnding       modulespecifiers.ImportModuleSpecifierEndingPreference `raw:"importModuleSpecifierEnding" config:"preferences.importModuleSpecifierEnding"`             // !!!
	AutoImportSpecifierExcludeRegexes []string                                               `raw:"autoImportSpecifierExcludeRegexes" config:"preferences.autoImportSpecifierExcludeRegexes"` // !!!
}

And, for prefs that are actually defined elsewhere (like the above), conversion works so long as the structure is the same, making this less error prone:

func (p *UserPreferences) ModuleSpecifierPreferences() modulespecifiers.UserPreferences {
	return modulespecifiers.UserPreferences(p.ModuleSpecifier)
}

UserPreferences are also now considered to be immutable. They should never be mutated for any reason. Copy() can be used if you need to make a deep copy. Copying is used for a test, but also for cloning the defaults to use during unmarshalling. Nothing else needs to mutate any preferences.

jakebailey avatar Dec 08 '25 17:12 jakebailey

I think I'm going to wait for the other two user pref related PRs to finish and just fix this PR instead of forcing that on everyone.

jakebailey avatar Dec 09 '25 22:12 jakebailey