Fix typings and add interfaces #20
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Added new `Settings` interface to define all the configuration settings and their typings. - Refactored the `Configuration::getConfigurationValue` method to use the new `Settings` interface and automatically get the correct typing of the specified key from the interface. This also enables intellisense support for the settings keys. It also removes the requirement of hardcoding the type via generic typing when using the method. ie. `getConfigurationValue<string[]>()`. - Removed all generic typings from the `getConfigurationValue` method calls.
- Fixed the typing of `overrideDefaultLanguageMultiLineComments` setting to use the `Record` utility type. The setting is defined in the package.json as an object, but it has always been wrongly typed as an array (`string[]`). The utility type `Record` defines the types for the keys and values, assuming all keys and all values will be of the same typing, ie stringed keys and numeric values, in our case though it's stringed values. It's an easier way of typing it as `{ [ key: string ]: string }`.
- Removed the import for the `SupportUnsupportedLanguages` interface, as this isn't available until 1.2.0. This was wrongly added to this PR while adding it for the 1.2.0 version.
- Removed the generic typing from the `Configuration::getConfigurationValue` method call in extension.ts file.
- Changed the job name in check-ts.yml from `deploy` to `check-ts`, as it's confusing seeing `deploy` on GitHub PR's and actions when it actually isn't the deploy workflow. This workflow only checks ts errors.
- Merge `ExtensionData::createExtensionData` method into `ExtensionData::setExtensionData` method for simplicity. We once had a unified method before until I split them into the 2 aforementioned methods in PR #12 (commit b0a40aa). Not sure why they were split, maybe because I thought too many `this.extensionData.set` was duplication, but it actually isn't. So we now merge them back.
- Added new `ExtensionMetaData` interface for better typings and vscode intellisense. - Changed the `extensionData` property map to use typing of the new `ExtensionMetaData` interface. - Changed `get` and `getAll` methods to use the typing of `ExtensionMetaData` interface. This is instead of using return type of the old `createExtensionData` method, and still allows vscode intellisense of the data properties. It also is a better way to type the properties.
- Refactor `Configuration::setBladeComments` method to remove the duplicated blade and html comment arrays. We now only define the arrays once as a variable and use it in the return statement. - Fix `setBladeComments` method return type to vscode's `CharacterPair` type. Also typed the blade and html comments variables as `CharacterPair`.
- Fixed the type of the `value` parameter of `Configuration::updateConfigurationValue` method to use `unknown` instead of `any`. This is because `any` type just disables TypeScript's error reporting, while `unknown` is a type-safe usage for "anything". Using `unknown` type is generally preferred over `any`.
- Fixed the type for the `config` param to vscode's `LanguageConfiguration` instead of `any` in both `setMultiLineCommentLanguageDefinitions` and `setSingleLineCommentLanguageDefinitions` methods. - Typed the `langArray` variable in `setMultiLineCommentLanguageDefinitions` method as `string[]`. - Fixed `defaultMultiLineConfig` variable's type to vscode's `LanguageConfiguration` instead of `any` in `setLanguageConfiguration`.
Previously, the `lineComment` style check in `setSingleLineCommentLanguageDefinitions` used the string `includes` method to determine if the style "included" a `;` because they could also be `;;`. But since those are the only 2 strings that it can be, it makes more sense to just do a strict equality check for both instead. - Refactored `lineComment` style conditional to remove the `includes` method and use strict equality checks for `;` OR `;;`.
…ons`
- Added a new custom type, `SingleLineCommentStyle`, in the new `commentStyles` file to define 3 union strings for the single line styles.
- Fix `style` variable type in `setSingleLineCommentLanguageDefinitions` to the union type of the new `SingleLineCommentStyle` type and `null`. This gives more info to ts for what `style` should be, either 1 of the 3 strings or `null`. By typing the 3 style options ("//", "#", ";"), we are also enabling vscode's intellisense to auto complete the value. This helps for future development.
- Changed the default value for `style` variable to be `null` instead of an empty string. This is so that we don't need to type the new `SingleLineCommentStyle` as `""` for empty string which looks weird as a union type. It also provides a better understanding of why the variable might be null, ie. an unsupported style.
Also changed the style empty string checks to null checks.
- Fixed typing for the language config `lineComment` property.
The property used to be solely a string, but as of June 2025, VScode changed it to be a string, object or null, in the PR microsoft/vscode#243283, specifically commit microsoft/vscode@d9145a2. However, this has not been updated in the npm package `@types/vscode` by DefinitelyTyped, so ts still thinks it should be a string only. So when we try to access the `comment` property in the object, then ts spits out errors.
While I created an issue for it (DefinitelyTyped/DefinitelyTyped#74372), we need need to manually add the changes until @types/vscode is changed accordingly.
- Added new `LineComment` custom type in the new commentStyles file to be a union of `string`, `LineCommentConfig`, and `null`. This typing is inline with vscode's change, but as a custom type instead of inside an interface. Also typed the `lineComment` variable as this new type and cast the `config.comments.lineComment` as this type too, to avoid any ts errors.
- Added new `LineCommentConfig` interface in the new commentStyles file, to provide info for the object options. This is inline with vscode's change.
- Changed the conditional for the `lineComment` as an object to first check it is of type object and not null before checking if `comment` property exists in the object. This prevents ts erroring.
- Fixed ts type error `Type 'void | CharacterPair' is not assignable to type 'CharacterPair'` while trying to set blade comments in `setLanguageConfiguration` method. This probably happens because the `setBladeComments` method is returning void and being set directly to the lang config `blockComment` property which doesn't work. So to avoid this, we set the blade comments into a new variable instead and then only set the `blockComment` property if the conditional check of `bladeComments` variable is truthy, ie. has any value except null,undefined, etc.
- Removed the unused duplicate `style` variable in `setSingleLineCommentLanguageDefinitions` method.
- Fixed the `data` param in logger's `debug` method to be an optional param, so it no longer is required if we just want the `DEBUG` message in the logs.
- Added an optional `namespace` property in the `ExtensionMetaData` interface. - Move the configuration namespace value of the `name` property in `setExtensionData` to the new `namespace` property as it's value. - Set the `name` property n `setExtensionData` to the name in the packageJson instead of the configuration namespace. - Change all occurrences of getting the `name` property to the new `namespace` property since each occurrence is for the namespace and not the actual name.
- Added new `ExtensionData::setExtensionDiscoveryPaths` method to set the paths to installed extensions, and added it's method call to the constructor. - Moved the setting of the extension paths from `setExtensionData` into the new `setExtensionDiscoveryPaths` method. - Added new `ExtensionPaths` interface and moved all the related properties from the `ExtensionMetaData` interface into the new interface. - Added new `ExtensionData::extensionDiscoveryPaths` property to hold the Map of all the paths, and uses the `ExtensionPaths` interface. - Changed references to the `extensionData` property in `setExtensionDiscoveryPaths` method to use the new `extensionDiscoveryPaths` property. - Added `getExtensionDiscoveryPath` and `getAllExtensionDiscoveryPaths` methods to get a specific extension path by key or get all paths, respectively. - Changed all method calls to `get` extension paths in `Configuration` class to use the `getExtensionDiscoveryPath` method; and added the `getAllExtensionDiscoveryPaths` method call in the constructor.
- Move the creation of the extension ID from the `getExtensionPackageJsonData` method into the `setExtensionData` method. And set the id property of the `extensionData` Map directly as the id variable. This is instead of using the `packageJSON` object as the middleman, and using a custom property which is then used to set the `extensionData` Map. The ID doesn't need to be in the packageJSON object, so we can just create and set the id directly in the `setExtensionData` method.
- Move `settingsNamespace` creation from `getExtensionPackageJsonData` method into the `setExtensionData` method. So that the `namespace` property of the `extensionData` Map can be set directly using the variable instead of using the `packageJSON` object as a middleman again. We only add namespace to `packageJSON.contributes.configuration` as a way to eventually add it to the `extensionData` Map. This is redundant code.
- Moved the check for package.json file existing (`fs.existsSync`) in `Configruation::readExtensionsFromDirectory` method into the `readJsonFile` utility function. This is so that the util function can check if a JSON file exists before trying to read it. If file doesn't exist then it just returns `null`.
- Removed the `fs.existsSync` check and replaced it with a `null` check in the `readExtensionsFromDirectory` method.
- Refactored `readJsonFile` util function to accept a new `throwOnFileMissing` param with a default value of `true`. This param, along with it's accompanying logic, will throw an error if a file is missing (doesn't exist) and when the param is `true, instead of returning `null`.
This allows only specific usages of the util function to return `null` instead of error throwing when the param is set to `false`. Thus being backward compatibile with other usages.
- Added new `extensionPath` private property to the `ExtensionData` class to hold the absolute path of the extension. - Move the `extensionPath` creation from `getExtensionPackageJsonData` method into the `constructor`, and set it to the new class property of the same name. - Split the extension package.json path creation from the `readJsonFile` method call param into a new separate `packageJSONPath` variable in the `getExtensionPackageJsonData` method, and use this new variable as the param of `readJsonFile`. - Added new `extensionPath` property to the `ExtensionMetaData` interface to allow the `extensionData` Map to hold this new property, and added setting it's value into the `setExtensionData` method. - Removed the setting of the `extensionPath` into the `packageJson` object in `getExtensionPackageJsonData`, in favour of the new `extensionData` Map property of the same name.
- Added the `throwOnFileMissing` param of the readJsonFile util function call to as `false` in the `getExtensionPackageJsonData` method to allow the util function to return `null`. - Changed return types of the `getExtensionPackageJsonData` function to allow `null`. - Added a `packageJson` null check in the constructor to only allow running the `setExtensionData` method when `packageJsonData` is not `null`.
- Added `packageJson` property to the `ExtensionMetaData` interface with the type of `IPackageJson`.
- Added a new `ExtensionMetaDataValue` utility type to automatically get all types of the `ExtensionMetaData` and construct them as a union type. This is needed because typing the Map value as `string` will no longer work with the new `packageJson` property since it's not a string.
- Changed the typing of the `extensionData` Map value to use the new `ExtensionMetaDataValue` type.
- Added new `extensionPath` param to the constructor so that the class can be initialised with an optional extension path. The param is `null` be default.
- Added the new `extensionPath` param variable to set the class's `extensionPath` property. Using a nullish coalescing operator (`??`) on the variable, we set the property to the variable if it's a string, otherwise set it to our extension's path.
- Changed setting of the extension namespace property to only be set if the extension is ours - wrapped the code in an if statement in `setExtensionData` method. This is because we only need to know our extension's config namespace.
- Set the new `packageJson` property into the `extensionData` Map with the class's `packageJsonData` property as the value in `setExtensionData`.
- Changed all typings of any variables relating to an array of extensions in `Configuration` class to use the `ExtensionMetaData` interface. Because we have the required properties in place now we don't need to type them as `any`, we can now use the interface for a better developing experience.
In addition, we can also now remove all occurrences of the array object typing of `Array<{ id: string; extensionPath: string; packageJSON: IPackageJson }>` and replace it with the `ExtensionMetaData` interface as the type `ExtensionMetaData[]`. This makes it easier to handle, and removes all duplication.
- Changed `Configuration::readExtensionsFromDirectory` method to use the `ExtensionData` class to `getAll` data of each extension in the directory. This removes all now redundant code dealing with the package.json within the method, since it's handled in the `ExtensionData` class.
- Changed `ExtensionData::getAll` method to return a plain object instead of a readonly Map, this is so that we can use it in the `Configuration::readExtensionsFromDirectory` method without ts errors.
Also added a conditional that will return null if the `extensionData` map has a size of 0, ie. is empty.
The `//TODO: REMOVE THIS BEFORE RELEASE. FOR TESTING ONLY.` comment along with it's testing `userExtensionsPath` variable code was mistakenly added in commit be9d972. This was never supposed have seen the lights of git, and is only used in development to ensure it gets the right extensions path otherwise the development environment would get the wrong path. Fixed by removing the testing code, and uncommenting the real code.
- Added some code comments for clarity. - Improved type safety on a few variables, including the `config` in the `Configuration::setLanguageConfigDefinitions` method and the `event` of the `onDidChangeConfiguration` API in the `activate` method in extension.ts.
By default `trailingComma` option is set to `all` when not defined, and this made changes to extension.ts file by adding unnecessary trailing commas to function and method params. This could be seen in all the `showInformationMessage` method calls. By defining the option with the value `es5`, trailing commas will only be affective on arrays and objects, not functions and methods.
- Added proper typings for json objects.
- `JsonObject` interface to define a generic json object.
- `JsonValue` type to define the possible types of the json values.
- `JsonArray` type to define an array in json.
- Added interfaces to define the single-line and multi-line language definitions, which extend the `JsonObject`: `SingleLineLanguageDefinitions` and `MultiLineLanguageDefinitions`.
- Refactored `readJsonFile` util function to use the new `JsonObject` interface and `JsonValue` type. The function now passes a generic type `T` as it's return value, if the generic type isn't defined when calling the function, then it defaults to `JsonObject`. This ensures that specific types or interfaces can be defined as the return type that naturally is or extends a `JsonObject`. Eg. `readJsonFile<IPackageJson>()`.
- Refactored `writeJsonFile` util function to pass a generic type `T` as it's return value and it's `data` param type as `T`.
- Changed `obj` param type in `reconstructRegex` util function to `unknown.
- Changed `convertMapToReversedObject` util function return type to `JsonObject`.
- Changed `readJsonFile` function call in `ExtensionData::getExtensionPackageJsonData` method to define the generic return type as `IPackageJson`.
- Changed `Configuration::getLanguagesToSkip` method return type to `JsonArray`.
- Refactored `Configuration::writeCommentLanguageDefinitionsToJsonFile` method to properly type the code as the new `MultiLineLanguageDefinitions` and `SingleLineLanguageDefinitions` interfaces.
- Changed `defaultMultiLineConfig` variable in `Configuration::setLanguageConfiguration` to be cast as `vscode.LanguageConfiguration` to ensure the `readJsonFile` return type is of that type.
- Auto-formatting changes., like removal of commas.
- Changed the `readJsonFile` function call return type in `Configuration::logDebugInfo` method to use the `MultiLineLanguageDefinitions` and `SingleLineLanguageDefinitions` interfaces.
- Fixed returns for `writeJsonFile` util function. In the previous commit (a832c7e), the return type was changed from `any` to a passable generic T type. But despite always being typed as returning a value even before the commit, the function doesn't actually return a value. It only performs a side effect by writing to disk. Fixed by removing the return type altogether and the docblock return comment. Also changed it's function calls in `Configuration::writeCommentLanguageDefinitionsToJsonFile` method to remove the passing of the generic return types. - Changed `data` param of `writeJsonFile` util function to be of type `JsonValue` instead of `T`. - Fixed `convertMapToReversedObject` util function return type to a passable generic `T` type, and that defaults to return of `JsonObject` if the type is not defined when calling. This keeps it constraint to `JsonObject` type while also accepting a passable return type that is JSONable. And changed the function calls in `Configuration::writeCommentLanguageDefinitionsToJsonFile` method to pass the `SingleLineLanguageDefinitions` interface as it's generic return type instead of casting it as such. - Removed the type from the `result` variable declaration in `convertMapToReversedObject` util function and we now cast it `as T` at the end when returning to ensure it satisfies the new return typing.
- Fixed `reconstructRegex` return type to explicitly be typed as `RegExp` and added docblock types.
- Fixed `mergeArraysBy` util function call in `Configuration::setLanguageConfiguration` method to pass its generic return type as vscode's `AutoClosingPair` or `OnEnterRule`. The function is declared with a passable generic `T` type, but it wasn't being used, so we might as well use it and pass the return types.
- Added `string` types to these class properties:
- `autoGeneratedDir`
- `singleLineLangDefinitionFilePath`
- `multiLineLangDefinitionFilePath`
- Added return types to these methods:
- `configureCommentBlocks`
- `registerCommands`
- `getOverriddenMultiLineComment`
- `getLanguageConfig`
- Fixed some docblock formatting and comments.
- Added `JsonObject` type to the variables `extensionsPaths` and `env` in `logDebugInfo` method.
- Changed various docblock comments.
- Changed Map key typings to the union type of `"customSupportedLanguages" | "supportedLanguages"` instead of just `string` in the class properties `singleLineBlocksMap` and `multiLineBlocksMap`.
This is to be more verbose for what keys the Map can have. Since it's only these 2 keys, it makes sense to just use them as a union type. It also aids in vscode intellisense.
- Changed typings to use `SingleLineCommentStyle` type instead of just `string` for:
- `singleLineBlocksMap` class property inner Map value.
- `getSingleLineLanguages` method return type.
- `tempMap` variable in `setSingleLineCommentLanguageDefinitions` method.
- Docblock types.
- `singleLineStyle` param of `setLanguageConfiguration` method.
- `style` variable in `handleSingleLineBlock` method.
- Added new `ExtraSingleLineCommentStyles` type to define all the extra single-line comment styles like `///` and `##`.
This is needed to prevent ts errors like: "error TS2322: Type '";;"' is not assignable to type 'SingleLineCommentStyle'."
This was because changing the style types to use `SingleLineCommentStyle`, it affected the `handleSingleLineBlock` method usage of setting the extra styles directly on the `style` variable. Thus ts would rightly error that we're not allowed to set the style to anything other than specified in the union type.
This new type is the union of all the extra styles, and we just use it on the `style` variable as a union of both `SingleLineCommentStyle` and `ExtraSingleLineCommentStyles` to ensure the extra styles are allowed.
- Fixed the docblock param `channel` to the actual name of the param `channelOverride`.
- Added new type `LanguageId` which is just an alias for `string`. By typing relevant strings as `LanguageId`, it becomes more self documenting.
- Changed `languageConfigFilePaths` and `languageConfigs` class properties to define it's types on the property instead of defining the generic type of the `new Map` instance. This is just to keep things looking clean and inline with other code.
- Changed `string` types to use the new `LanguageId` in `Configuration` class for:
- `languageConfigFilePaths`, `languageConfigs`, `singleLineBlocksMap`, and `multiLineBlocksMap` class property Maps.
- Docblock types.
- `isLangIdDisabled`, `isLangIdMultiLineCommentOverridden`, `getOverriddenMultiLineComment` and `getLanguageConfig` method params.
- Various variables in many methods.
- `getMultiLineLanguages` and `getSingleLineLanguages` method returns.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request refactors and improves type safety, code clarity, and configuration handling in the
Configurationclass and related files. The most significant changes include replacing generic types with more precise interfaces, updating configuration access patterns, and cleaning up code for better maintainability and correctness.Type Safety and Interface Improvements
Settings,ExtensionMetaData,SingleLineLanguageDefinitions,MultiLineLanguageDefinitions, etc.) instead of generic types orany, improving type safety throughoutsrc/configuration.ts.Settingsinterface, ensuring type safety and reducing the risk of runtime errors.Configuration and Extension Data Handling
ExtensionDataclass, simplifying extension data collection and reducing direct file system operations.Language Configuration Logic
Miscellaneous Improvements
deploytocheck-tsfor clarity.This pull request refactors how configuration values are accessed in theConfigurationclass to improve type safety and maintainability. The main change is the introduction of a newSettingsinterface that defines all supported configuration keys and their types, and the update of thegetConfigurationValuemethod to use this interface for type-safe access throughout the codebase.Type Safety and Configuration Refactor:
Settingsinterface insrc/interfaces/settings.tsthat explicitly defines all available configuration keys and their respective types.getConfigurationValuemethod insrc/configuration.tsto use generic key access (K extends keyof Settings) and return the correct type fromSettings, ensuring type-safe retrieval of configuration values.getConfigurationValueinsrc/configuration.tsto remove explicit type arguments and leverage the new type-safe method signature.