chore(suite-desktop-core): typescriptize electron builder hooks

[Lemonexe]

Description

Actually use TypeScript for electron-builder hooks - currently they have .ts extension, but are written plain JavaScript (CJS, as electron-builder needs), and type checking never ever happens.

• rewrite CJS to TS (ESM)
• include scripts to type-check. We don't do it for most JS scripts, but these scripts are built around API of electron-builder & electron/notarize, and IMO they'd really benefit from being typechecked
  • and indeed, there is inconsistent TS in notarize.ts
    • probably when @electron/notarize was bumped by yours truly, but I couldn't have noticed when I believed it was TS, yet didn't know it's not really TS 😅
• add build:lib to compile them to CJS for electron-builder right before building
• fix eslint

Related Issue

Followup to https://github.com/trezor/trezor-suite-private/issues/16 and #17050

[Lemonexe]

This fn was async (and has to stay, because TS requires the hook to return Promise), but without await, which failed eslint, hence eslint disable 🙈

👉 needs to be verified if changing execSync to exec doesn't break the signing with codesign build.
But should be ok. This is the code that invokes it. It awaits the Promise, so LGTM..

[Lemonexe]

UPDATE: This was actually wrong 😱

[Lemonexe]

This includes the scripts to type-check, which was not done before, but does not build them, that's what build:lib does (tsconfig.lib.json)

[Lemonexe]

funnily enough, eslint tolerates CJS exports.default in a TS file, but errors at ES export default.

[Lemonexe]

No type check ever happened on this "typescript" file.
TS would fail, because at the very least, TS requires (context: any) =>, but then it's no longer valid JS (electron-builder crashes)

[coderabbitai]

Walkthrough

This pull request comprises modifications across configuration files, build scripts, and source files in the suite-desktop-core and suite-desktop projects. The ESLint configuration has been updated by adding a new rule object specifically targeting files in the scripts directory, thereby disabling specific linting rules. The package.json file has been modified to introduce a new build script (build:lib) and to update the build flow in the build:core script to depend on it. Several scripts (notarize.ts, setElectronFuses.ts, and sign-windows.ts) have been refactored from CommonJS to ES modules, altering import/export syntax and type annotations. The TypeScript configuration has been expanded by including the scripts directory in tsconfig.json and adding a new configuration file (tsconfig.lib.json) for library builds. Additionally, references in the Electron builder configuration have been updated to point to the corresponding JavaScript files.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3340814 and 902838b.

📒 Files selected for processing (8)

• packages/suite-desktop-core/eslint.config.mjs (1 hunks)
• packages/suite-desktop-core/package.json (1 hunks)
• packages/suite-desktop-core/scripts/notarize.ts (2 hunks)
• packages/suite-desktop-core/scripts/setElectronFuses.ts (2 hunks)
• packages/suite-desktop-core/scripts/sign-windows.ts (1 hunks)
• packages/suite-desktop-core/tsconfig.json (1 hunks)
• packages/suite-desktop-core/tsconfig.lib.json (1 hunks)
• packages/suite-desktop/electron-builder-config.js (2 hunks)

✅ Files skipped from review due to trivial changes (1)

• packages/suite-desktop-core/tsconfig.lib.json

🔇 Additional comments (14)

packages/suite-desktop-core/tsconfig.json (1)

8-13: Enhance Type Checking by Including the "scripts" Directory.
The updated "include" array now encompasses the "scripts" folder, ensuring that TypeScript will type-check these files. This is a valuable improvement in alignment with the PR objective to transition electron-builder hooks to TypeScript and boost overall type safety.

packages/suite-desktop/electron-builder-config.js (2)

165-166:

Details

❓ Verification inconclusive

Verify the existence of the compiled hook scripts.

The paths for both afterPack and afterSign hooks have been updated to point to the compiled JavaScript files. Let's verify their existence and correct placement.

---

🏁 Script executed:

#!/bin/bash
# Description: Verify the existence of the compiled hook scripts and their correct placement.

# Test: Check if the compiled JavaScript files exist
fd --type f "setElectronFuses.js|notarize.js" ../suite-desktop-core/lib

# Test: Check if the TypeScript source files exist
fd --type f "setElectronFuses.ts|notarize.ts" ../suite-desktop-core/scripts

Length of output: 381

---

Attention: Verify Compiled Hook Scripts & Update Directory Paths if Needed

The automated check did not find the expected directories at the relative paths (../suite-desktop-core/lib and ../suite-desktop-core/scripts). This could mean that either the compiled hook scripts are in a different location (for example, under packages/suite-desktop-core/lib) or the relative path in electron-builder-config.js needs to be updated to match the current repository structure.

• Action Items:
  • Confirm if the compiled hook scripts (setElectronFuses.js and notarize.js) are located in a different directory (likely within the packages/suite-desktop-core folder).
  • Update the paths in packages/suite-desktop/electron-builder-config.js accordingly if the structure has changed.
  • Perform a manual verification of the existence and correct placement of these files if the automated search cannot resolve the directories.

---

137-137:

Details

❓ Verification inconclusive

Verify the existence of the compiled Windows signing script.

The path has been updated to point to the compiled JavaScript file. Let's verify its existence and correct placement.

---

🏁 Script executed:

#!/bin/bash
# Description: Verify the existence of the compiled Windows signing script and its correct placement.

# Test: Check if the compiled JavaScript file exists
fd --type f "sign-windows.js" ../suite-desktop-core/lib

# Test: Check if the TypeScript source file exists
fd --type f "sign-windows.ts" ../suite-desktop-core/scripts

Length of output: 345

---

Action: Verify the placement of the compiled signing script

It appears that the initial verification command used an incorrect relative path from the repository root. Please confirm manually that the compiled file is in the proper location. For example, from the repository root you can run:

fd --type f "sign-windows.js" packages/suite-desktop-core/lib

Ensure that the file exists at that location (which corresponds to the relative path ../suite-desktop-core/lib/sign-windows.js from packages/suite-desktop/electron-builder-config.js). If the file is missing or not found in the expected directory, please review the build output and TypeScript compilation settings.

packages/suite-desktop-core/eslint.config.mjs (1)

23-29: LGTM! The ESLint configuration aligns with the PR objective.

The new configuration for the scripts directory appropriately disables:

• no-console: Reasonable for build scripts that need to log progress.
• import/no-default-export: Required for compatibility with electron-builder hooks.

packages/suite-desktop-core/scripts/notarize.ts (3)

1-2: LGTM! Clean transition to ES module syntax.

The imports are correctly updated to use ES module syntax and include proper type imports.

---

4-4: LGTM! Type-safe hook signature.

The hook is correctly typed as Hooks['afterSign'] and exported using ES module syntax.

Also applies to: 31-31

---

27-28:

Details

✅ Verification successful

Verify the TypeScript issue with notarize tool.

The TODO comment indicates a known TypeScript issue with the notarize tool's function overloads.

---

🌐 Web query:

Are there any known TypeScript issues with @electron/notarize function overloads?

💡 Result:

Based on the search results provided, there are no direct mentions of known TypeScript issues specifically with @electron/notarize function overloads. However, several general limitations of TypeScript function overloading apply universally and could theoretically affect any implementation using this pattern:

1. Type Safety Concerns
   TypeScript function overloads aren't fully type-safe at implementation level[3][7]. The compiler doesn't verify that implementation signatures satisfy all overload cases[3][7], creating potential for runtime errors if return types don't match declared overload signatures.

2. Implementation Complexity
   Overloaded functions require a single implementation handling all cases through manual type checks[1][4][7]. This can lead to convoluted code when handling multiple parameter combinations[1][4], particularly in complex APIs like those found in Electron tooling.

3. Signature Maintenance
   Adding/modifying overload signatures requires careful coordination with implementation logic[4][7]. Mismatches between declared signatures and actual implementation can occur silently[3][7].

While none of the Electron-specific search results ([6][8][9][10]) explicitly mention overload-related issues, they focus on configuration problems with macOS notarization rather than TypeScript typing concerns. The general TypeScript limitations above would apply to any project using function overloading patterns, including potential Electron tooling implementations.

For Electron notarization specifically, configuration issues appear more common than typing problems based on:

• Environment variable setup[10]
• Notarytool compatibility[9]
• Build process integration[6]

No results indicate @electron/notarize itself has known overload-specific defects - any issues would likely stem from general TypeScript overload limitations rather than package-specific bugs[1][3][4][7].

Citations:

• 1: https://aaronbos.dev/posts/function-overload-typescript
• 2: https://dev.to/thinkster/typescript-function-overloading-k9i
• 3: https://michaelrutter.dev/conditional-types-and-overloading/
• 4: https://www.bennadel.com/blog/3339-using-method-and-function-overloading-in-typescript.htm
• 5: https://kentcdodds.com/blog/define-function-overload-types-with-type-script
• 6: skipped macOS notarization reason=notarize options were not provided electron/notarize#175
• 7: https://javascript.plainenglish.io/typescript-the-problem-with-function-overloads-017f8bd2c78d
• 8: https://github.com/electron/notarize
• 9: 'tool: "notarytool"' won't work electron-userland/electron-builder#7677
• 10: Notarize notarytool JSON parsing error electron-userland/electron-builder#8258

---

Notarize Type Overload Verification: No Package-Specific Issue Detected

After reviewing the issue, it appears that the TypeScript error arises from general limitations in handling function overloads rather than a bug in the @electron/notarize package. The usage of as any to bypass the overload matching is an acceptable temporary workaround until TypeScript’s type checking improves. No additional changes are required in this case.

packages/suite-desktop-core/scripts/sign-windows.ts (2)

1-3: LGTM! Clean transition to ES module syntax with proper typing.

The hook is correctly typed as CustomWindowsSign and uses modern arrow function syntax.

---

16-21: LGTM! Safe transition to async execution.

The change from execSync to exec is safe as electron-builder awaits the Promise returned by the hook.

packages/suite-desktop-core/scripts/setElectronFuses.ts (3)

1-3: LGTM! Clean transition to ES module syntax.

The imports are correctly updated to use ES module syntax and include proper type imports.

---

6-10: LGTM! Type-safe platform map.

The as const assertion ensures type safety by treating the platform extensions as literal types.

---

12-12: LGTM! Type-safe hook signature.

The hook is correctly typed as Hooks['afterPack'] and exported using ES module syntax.

Also applies to: 43-43

packages/suite-desktop-core/package.json (2)

10-10: New "build:lib" Script Added for TypeScript Library Building

The addition of the "build:lib" script, which cleans the ./lib directory and builds the TypeScript project using tsconfig.lib.json, is a good enhancement. This enables type checking and builds specifically for the library modules. Please verify that tsconfig.lib.json correctly includes the necessary files (e.g., the ./scripts directory) and compiler options to meet your project requirements.

---

11-11: Integration of "build:lib" in the "build:core" Script

Updating the "build:core" script to invoke "yarn build:lib" before cleaning the dist directory and triggering the Webpack build ensures that the library build is a prerequisite for the core build. This sequential dependency helps catch type errors and issues early. Double-check that the output from the lib build is correctly used by your core modules during the Webpack process.

✨ Finishing Touches

• 📝 Generate Docstrings (Beta)

---

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?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[peter-sanderson]

Very nice, much types, such typescript 🐶

👏 Good job!

(haven't tested tho)