Migrate Build to ESM #291
Migrate Build to ESM #291
Conversation
WalkthroughThe changes encompass updates to configuration files for ESLint and Mocha, the introduction of a tailored ESLint setup for TypeScript and Vue.js, and various code improvements across multiple components and service files. These enhancements include modifications to type declarations, variable scopes, error handling, and formatting adjustments for better readability. The TypeScript configuration has also been revised to support ECMAScript modules, reflecting a commitment to modern JavaScript practices. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant S as Service
participant T as Test Suite
participant L as Linter
C->>L: Request linting
L->>C: Return linting results
C->>S: Make API call
S->>C: Return data
T->>S: Execute tests
S->>T: Return test results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (31)
- frontend/.vscode/settings.json (1 hunks)
- frontend/eslint.config.js (1 hunks)
- frontend/package.json (4 hunks)
- frontend/src/common/auditlog.ts (2 hunks)
- frontend/src/common/backend.ts (6 hunks)
- frontend/src/common/crypto.ts (2 hunks)
- frontend/src/common/jwe.ts (3 hunks)
- frontend/src/common/jwt.ts (2 hunks)
- frontend/src/common/updatecheck.ts (1 hunks)
- frontend/src/common/userdata.ts (2 hunks)
- frontend/src/common/util.ts (3 hunks)
- frontend/src/common/wot.ts (2 hunks)
- frontend/src/components/AdminSettings.vue (2 hunks)
- frontend/src/components/AuditLogDetailsSignedWotId.vue (1 hunks)
- frontend/src/components/DeviceList.vue (1 hunks)
- frontend/src/components/GrantPermissionDialog.vue (2 hunks)
- frontend/src/components/NavigationBar.vue (3 hunks)
- frontend/src/components/SignUserKeysDialog.vue (3 hunks)
- frontend/src/components/TrustDetails.vue (1 hunks)
- frontend/src/components/UserProfile.vue (1 hunks)
- frontend/src/components/UserkeyFingerprint.vue (2 hunks)
- frontend/src/components/VaultDetails.vue (2 hunks)
- frontend/src/components/VaultList.vue (1 hunks)
- frontend/src/i18n/index.ts (1 hunks)
- frontend/src/router/index.ts (2 hunks)
- frontend/src/shims-vue.d.ts (1 hunks)
- frontend/test/common/crypto.spec.ts (5 hunks)
- frontend/test/common/jwe.spec.ts (2 hunks)
- frontend/test/common/jwt.spec.ts (2 hunks)
- frontend/test/common/util.spec.ts (1 hunks)
- frontend/tsconfig.json (1 hunks)
Files skipped from review due to trivial changes (16)
- frontend/src/common/updatecheck.ts
- frontend/src/common/userdata.ts
- frontend/src/common/wot.ts
- frontend/src/components/AdminSettings.vue
- frontend/src/components/AuditLogDetailsSignedWotId.vue
- frontend/src/components/DeviceList.vue
- frontend/src/components/SignUserKeysDialog.vue
- frontend/src/components/TrustDetails.vue
- frontend/src/components/UserProfile.vue
- frontend/src/components/UserkeyFingerprint.vue
- frontend/src/components/VaultList.vue
- frontend/src/i18n/index.ts
- frontend/src/router/index.ts
- frontend/src/shims-vue.d.ts
- frontend/test/common/jwt.spec.ts
- frontend/test/common/util.spec.ts
Additional context used
Biome
frontend/src/common/util.ts
[error] 47-47: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
Additional comments not posted (39)
frontend/tsconfig.json (2)
17-18: Ensure compatibility with ESM settings.The addition of
"esm": trueand"experimentalSpecifierResolution": "node"in thets-nodeconfiguration is crucial for ESM support. Verify that all scripts and modules are compatible with these settings.
20-20: Include test files in TypeScript compilation.The inclusion of
test/**/*.tsin theincludearray is a positive step towards ensuring that test files are compiled and checked for type safety.frontend/.vscode/settings.json (2)
23-26: Update Mocha settings for ESM compatibility.The
mochaExplorer.nodeArgvsettings enable ES module support and suppress experimental warnings, aligning with the transition to ESM.
30-31: Adopt the new ESLint flat configuration.The addition of
eslint.useFlatConfig: trueindicates a shift to the new ESLint configuration format, which is recommended for modern JavaScript projects.frontend/package.json (4)
2-2: Transition to ECMAScript modules.The addition of
"type": "module"signifies a shift to ESM, affecting how modules are imported and exported. Ensure all files are compatible with this change.
13-13: Simplify ESLint configuration.The removal of the configuration file reference in the
"lint"script suggests reliance on default or new configuration methods, aligning with the move to a flat ESLint configuration.
23-23: Update Mocha loader for ESM.The change to
"loader": "ts-node/esm"in the Mocha configuration supports the use of ES modules in tests, enhancing compatibility with the new module system.
40-50: Update dependencies to latest versions.The updates to
chai,chai-as-promised,eslint,eslint-plugin-vue, and the addition oftypescript-eslintreflect a move to modernize the codebase and ensure compatibility with ESM.frontend/src/common/jwt.ts (2)
21-21: Improved type safety inbuildmethod.Changing the
payloadparameter toobjectenhances type safety by ensuring it is always an object.
49-49: Improved type safety inparsemethod.Changing the return type to
objectenhances type safety and aligns with thebuildmethod.frontend/eslint.config.js (1)
1-97: Comprehensive ESLint configuration added.The new ESLint configuration is well-structured and aligns with TypeScript and Vue.js best practices.
frontend/src/common/util.ts (2)
6-6: Enhanced type safety intransactionmethod.Removing the default type of
anyenforces stricter type requirements, improving type safety.
26-26: Improved type handling inDeferredclass.Changing the
rejectmethod's parameter type tounknownpromotes better type handling.frontend/src/common/auditlog.ts (2)
119-119: Enhance type safety with specific function type.The change from
Functionto() => voidfor thedebouncedResolvePendingEntitiesparameter improves type safety by ensuring that only functions with no parameters and no return value are accepted.
136-136: Improve readability with destructuring syntax.The change from
([_, v])to([, v])in thefiltermethod improves readability by omitting the unused variable name.frontend/src/components/NavigationBar.vue (4)
71-71: Update icon name for clarity.The renaming of
ArrowRightOnRectangleIcontoArrowRightStartOnRectangleIconreflects a change in the icon's designation, improving clarity without altering functionality.
72-72: Add explicit type definition for FunctionalComponent.The addition of
FunctionalComponentin the import statements indicates a shift towards more explicit type definitions, enhancing type safety.
83-83: IntroduceProfileDropdownItemtype for clarity.The new
ProfileDropdownItemtype enhances type safety and clarity by defining the structure of items in the profile dropdown.
104-104: UpdateprofileDropdowntype for improved type safety.Changing the type of
profileDropdownfromany[]toProfileDropdownItem[][]improves type safety and ensures alignment with the newly defined type.frontend/src/components/GrantPermissionDialog.vue (1)
124-124: Useconstfor non-reassigned variable.Changing
tokensfromlettoconstclarifies that the variable is not reassigned, preventing potential bugs and improving code clarity.frontend/test/common/jwe.spec.ts (3)
3-3: LGTM: Use ofwebcryptoimport.The change to import
webcryptofromnode:cryptoaligns with ESM standards and modern JavaScript practices.
10-10: LGTM: Setting globalcryptowithwebcrypto.The use of
webcryptoto set the globalcryptoobject is appropriate for the test environment.
25-25: LGTM: Use ofconstforderivedKey.Changing
derivedKeytoconstenforces immutability, which is a best practice.frontend/src/common/jwe.ts (3)
84-84: LGTM: Use of generic return type indecryptEcdhEs.The change to a generic return type
Promise<T>enhances type safety by allowing the caller to specify the expected type.
99-99: LGTM: Use of generic return type indecryptPbes2.The change to a generic return type
Promise<T>enhances type safety by allowing the caller to specify the expected type.
113-113: LGTM: Use of generic return type indecrypt.The change to a generic return type
Promise<T>enhances type safety by allowing the caller to specify the expected type.frontend/src/common/backend.ts (6)
30-30: LGTM: Simplified error handling inaxiosAuthinterceptor.The direct throw of
UnauthorizedErrorsimplifies the error handling logic.
163-163: LGTM: Use ofconstfordateString.Changing
dateStringtoconstenforces immutability, which is a best practice.
231-231: LGTM: Updated return type inremoveDevice.The change to
Promise<AxiosResponse<unknown>>enhances type safety.
236-236: LGTM: Updated return type inputDevice.The change to
Promise<AxiosResponse<unknown>>enhances type safety.
294-294: LGTM: Use ofconstforcfg.Changing
cfgtoconstenforces immutability, which is a best practice.
411-411: LGTM: Updated return type inrethrowAndConvertIfExpected.The change to
neverclarifies that the function will always throw an error, enhancing type safety.frontend/test/common/crypto.spec.ts (4)
4-4: LGTM! Modern import forwebcrypto.The import statement for
webcryptofromnode:cryptois a modern and cleaner approach.
39-39: LGTM! Simplified setup ofglobal.crypto.The
beforehook now directly uses the importedwebcrypto, streamlining the setup process.
44-45: LGTM! Use ofconstfor immutable variables.Changing
ecdhP384,ecdsaP384, andrecoveryKeytoconstenhances code safety by indicating immutability.Also applies to: 65-65
244-244: LGTM! Refined type definition forinput.The type refinement for
inputtoJsonWebKey & Record<string, string>improves type safety and clarity.frontend/src/common/crypto.ts (1)
6-8: LGTM! Enhanced type safety forUnwrapKeyError.Changing
actualErrortounknownimproves type safety by requiring explicit handling.frontend/src/components/VaultDetails.vue (2)
367-367: LGTM! Enhanced type safety forisAuthorityDto.Changing the parameter type to
unknownensures stricter type safety.
394-394: LGTM! Improved readability inaddAuthorityBackend.The added whitespace around the
switchstatement enhances readability.
There was a problem hiding this comment.
Looks good to me but npm outdated shows still some outdated packages (Keycloak is fixed on the dev branch, so this can be skipped), is there a reason for it?
Package Current Latest Location Depended by
@rollup/rollup-linux-x64-gnu 4.20.0 4.21.0 node_modules/@rollup/rollup-linux-x64-gnu frontend
@types/node 20.14.15 22.5.0 node_modules/@types/node frontend
keycloak-js 25.0.2 25.0.4 node_modules/keycloak-js frontend
vite 5.4.0 5.4.2 node_modules/vite frontend
vue-i18n 9.13.1 9.14.0 node_modules/vue-i18n frontend
Except for node (which I didn't update because I didn't want to touch GitHub Actions as well) and keycloak (which should probably match the kc version we ship) the reason simply is that they all outpaced this PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
CHANGELOG.md (1)
Line range hint
17-22: Correct the typographical error in the "Changed" section.The word "Bumpd" should be corrected to "Bumped."
Apply this diff to correct the typo:
- Bumpd build time dependencies + Bumped build time dependencies
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- frontend/package.json (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/package.json
Additional comments not posted (3)
CHANGELOG.md (3)
Line range hint
9-12: The "Added" section is clear and concise.The entries provide a good overview of new features related to user key management and identity verification.
24-24: The "Fixed" section is clear and concise.The entry provides a concise description of the accessibility improvement.
Line range hint
26-30: The "Security" section is clear and specific.The entries provide specific details about the vulnerabilities addressed in the update.
This PR bumps various build-time dependencies and completes the transitions to ESM.
ES modules is the de-facto standard adopted by all major frameworks and tools in recent years. In order to keep our dependencies up to date, we had to migrate them sooner or later.
vite.jshas been spitting out deprecation warnings for some time alreadychaiversion 5 dropped support for other module systemseslintis urging users to adopt the Flat Configuration Format with plans to drop the older.eslintrc.jsformat, which is incompatible with ESMThis PR also adjusts VSCode settings for it to pick up the new linter and test configurations.
Last but not least, plenty of linter warnings/errors have been fixed, as it seems like the linter has not been run recently.