Skip to content

Conversation

@yCodeTech
Copy link
Owner

This pull request refactors how configuration values are accessed in the Configuration class to improve type safety and maintainability. The main change is the introduction of a new Settings interface that defines all supported configuration keys and their types, and the update of the getConfigurationValue method to use this interface for type-safe access throughout the codebase.

Type Safety and Configuration Refactor:

  • Introduced a new Settings interface in src/interfaces/settings.ts that explicitly defines all available configuration keys and their respective types.
  • Updated the getConfigurationValue method in src/configuration.ts to use generic key access (K extends keyof Settings) and return the correct type from Settings, ensuring type-safe retrieval of configuration values.
  • Refactored all usages of getConfigurationValue in src/configuration.ts to remove explicit type arguments and leverage the new type-safe method signature.

- 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.
@yCodeTech yCodeTech added the enhancement New feature or quality of life enhancement label Jan 11, 2026
- 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or quality of life enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants