Skip to content

Conversation

@ysmoradi
Copy link
Member

@ysmoradi ysmoradi commented Oct 31, 2025

closes #11542

Summary by CodeRabbit

  • Chores
    • Enhanced JavaScript bundling and build process with esbuild integration for improved performance.
    • Reorganized TypeScript module structure to improve code maintainability and clarity.
    • Updated TypeScript configuration for better module resolution.

@ysmoradi ysmoradi requested review from Copilot and msynk October 31, 2025 14:14
@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Restructures the TypeScript module system from single-file bundled output to an ES module-based architecture. Centralizes type declarations in a dedicated typings.d.ts file, exports classes as modules, introduces index.ts as the entry point for coordinating initialization and global window attachment, and updates the build pipeline with esbuild bundling.

Changes

Cohort / File(s) Summary
Build Configuration
src/Templates/Boilerplate/Bit.Boilerplate/.gitignore, src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Boilerplate.Client.Core.csproj
Adds ignore pattern for compiled JS files and introduces esbuild bundling step (executing after TypeScript compilation) to bundle Scripts/index.js into wwwroot/scripts/app.js, with conditional minification for production.
Type Declarations
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/typings.d.ts
New centralized declaration file introducing DotNetObject interface, BitButil class with webAuthn methods, and BitTheme configuration types (BitThemeOptions, onThemeChangeType).
Entry Point & Initialization
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/index.ts
New entry point that aggregates core modules (bswup, theme, events, App, WebInteropApp, optionally Ads) and attaches selected classes to the global window object.
Script Classes (Module Exports)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/Ads.ts, App.ts, WebInteropApp.ts
Converts classes from implicit global exposure to explicit ES module exports; removes individual global window assignments and inline type declarations.
Module Dependencies
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/bswup.ts, events.ts
Adds explicit imports for App module to replace implicit global access.
Theme Module
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/theme.ts
Removes inline type declarations (BitTheme, BitThemeOptions, onThemeChangeType), which are now centralized in typings.d.ts; retains runtime initialization logic.
TypeScript Configuration
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/tsconfig.json
Adds "module": "es2015" for ES module output, replaces "outFile" with "moduleResolution": "node" to enable proper module path resolution.

Sequence Diagram

sequenceDiagram
    actor Build
    participant tsc as TypeScript Compiler
    participant esbuild as esbuild Bundler
    participant index as Scripts/index.ts
    participant app as App Module
    participant webinterop as WebInteropApp Module
    participant ads as Ads Module (conditional)
    participant window as Global Window

    Build->>tsc: Compile TypeScript files
    tsc->>tsc: Output ES modules
    Build->>esbuild: Bundle index.js
    esbuild->>index: Load entry point
    index->>app: import App
    index->>webinterop: import WebInteropApp
    alt Ads enabled
        index->>ads: import Ads
    end
    index->>window: app.App = App
    index->>window: window.WebInteropApp = WebInteropApp
    alt Ads enabled
        index->>window: window.Ads = Ads
    end
    esbuild->>esbuild: Output app.js bundle
    esbuild-->>Build: Complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring attention:

  • typings.d.ts: Verify that centralized type definitions match the removed inline declarations and correctly represent Blazor interop and Bit.Butil APIs.
  • index.ts: Confirm that module initialization order is correct and conditional Ads import logic properly handles the template directive.
  • Build configuration changes: Ensure esbuild bundling step correctly processes the new ES module output and produces the expected app.js artifact; validate that minification still functions in production builds.
  • Module imports in bswup.ts and events.ts: Verify that explicit App imports replace implicit global access without breaking runtime behavior.

Poem

🐰 Modules now dance, no longer alone,
Types in typings find their true home,
Index guides them with care to the stage,
Each export shines on a modern-bound page,
From scattered to structured, ES modules reign! 📦✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues Check ✅ Passed The pull request successfully addresses all four primary objectives from issue #11542. First, proper module output configuration is implemented through tsconfig.json updates that add "module": "es2015" and "moduleResolution": "node" while removing the single-file outFile approach [Objective 4]. Second, centralized TypeScript type declarations are established in a new typings.d.ts file containing DotNetObject, BitButil, BitTheme, BitThemeOptions, and onThemeChangeType, eliminating scattered inline declarations [Objective 2]. Third, a new index.ts entry-point module is created that coordinates imports and initialization of core application scripts [Objective 3]. Fourth, the App, WebInteropApp, and Ads classes are converted to proper ES module exports and registered globally through the index.ts entry point [Objective 1]. The supporting infrastructure changes (esbuild bundling in csproj and .gitignore updates) enable the module system to function properly during the build process.
Out of Scope Changes Check ✅ Passed All changes in this pull request are directly related to implementing the proper module system specified in issue #11542. The .gitignore additions for JavaScript files support the new bundling workflow, and the csproj modifications adding esbuild bundling configuration provide the necessary infrastructure for the module system to function during the build process. The TypeScript file modifications (adding exports, removing global assignments, adding imports, creating the entry point) all directly support the module system implementation. There are no unrelated changes or improvements introduced outside the scope of establishing proper TypeScript module organization and configuration.
Title Check ✅ Passed The pull request title "Implement proper js module system for bit Boilerplate (#11542)" directly and accurately reflects the main objective and changes in the changeset. The modifications across multiple files—including conversion of classes to proper ES exports, introduction of a new entry point (index.ts), centralization of type declarations (typings.d.ts), and updates to tsconfig.json for module resolution—all work together to implement a proper module system. The title is clear, concise, specific to the core change, and free of vague terminology, making it easy for reviewers scanning project history to understand the primary purpose.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the TypeScript build system from a single-file compilation approach to a module-based architecture with bundling. The changes enable proper ES module syntax with imports/exports while maintaining the same runtime behavior through esbuild bundling.

  • TypeScript files now use ES module syntax with import/export statements
  • Shared type definitions moved to a central typings.d.ts file
  • Build process updated to compile TypeScript to individual modules, then bundle with esbuild

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tsconfig.json Updated to use ES2015 modules with node resolution instead of outFile compilation
typings.d.ts New file centralizing type definitions for Blazor, BitButil, and BitTheme
theme.ts Removed inline type declarations now defined in typings.d.ts
index.ts New entry point that imports all modules and exposes classes to window global
events.ts Added import for App class
bswup.ts Added import for App class
WebInteropApp.ts Changed to export class, removed inline BitButil types and window assignment
App.ts Changed to export class, removed inline DotNetObject type and window assignment
Ads.ts Changed to export class, removed window assignment
Boilerplate.Client.Core.csproj Added esbuild bundling step to create final app.js from compiled modules
.gitignore Added pattern to ignore intermediate .js files in Scripts folder

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/typings.d.ts (1)

11-12: Consider more specific types for WebAuthn methods.

The BitButil.webAuthn methods currently use unknown for both parameters and return types. While this is safe, it limits type checking and IDE assistance. If the WebAuthn API shapes are stable, consider defining specific interfaces for credential options and responses.

Example refactor:

// Bit.Butil types:
interface WebAuthnCredentialOptions {
    challenge: BufferSource;
    // ... other known properties
}

interface WebAuthnCredential {
    id: string;
    type: string;
    // ... other known properties
}

declare class BitButil {
    static webAuthn: {
        getCredential: (options: WebAuthnCredentialOptions) => Promise<WebAuthnCredential>,
        createCredential: (options: WebAuthnCredentialOptions) => Promise<WebAuthnCredential>
    }
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 28fe8bb and d1dc4dc.

📒 Files selected for processing (11)
  • src/Templates/Boilerplate/Bit.Boilerplate/.gitignore (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Boilerplate.Client.Core.csproj (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/Ads.ts (2 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/App.ts (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebInteropApp.ts (2 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/bswup.ts (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/events.ts (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/index.ts (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/theme.ts (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/typings.d.ts (1 hunks)
  • src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/tsconfig.json (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: CodeQL analysis (javascript-typescript)
  • GitHub Check: build and test
🔇 Additional comments (20)
src/Templates/Boilerplate/Bit.Boilerplate/.gitignore (1)

229-229: LGTM! Properly excludes generated intermediate files.

The new ignore pattern correctly excludes TypeScript-generated JavaScript files in the Scripts folder, which are intermediate artifacts before esbuild bundles them into the final app.js.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/Ads.ts (1)

4-4: LGTM! Proper module export pattern.

The class is now exported as a module, and global window exposure is correctly delegated to the index.ts entry point where it's conditionally exposed based on the ads template directive.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/bswup.ts (1)

3-3: LGTM! Correct module import.

The import is necessary since App is no longer globally attached to window. The named import syntax correctly matches the export in App.ts.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/events.ts (1)

3-3: LGTM! Correct module import.

The import enables module-based access to App.publishMessage at line 21, replacing the previous reliance on global window attachment.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/tsconfig.json (1)

6-9: LGTM! Critical configuration for ES module support.

These changes directly address the PR objectives:

  • "module": "es2015" enables modern ES module imports/exports between TypeScript files
  • "moduleResolution": "node" allows TypeScript to reliably resolve imports
  • Removing outFile shifts bundling responsibility to esbuild in the build pipeline

This is the foundation for the proper module system implementation.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/App.ts (2)

3-3: LGTM! Proper module export.

The App class is now correctly exported as a module, with global window exposure delegated to index.ts.


5-7: DotNetObject type is properly declared.

The verification confirms that DotNetObject is correctly declared in typings.d.ts at line 2 with declare interface DotNetObject {. The type is available for use in App.ts and there are no issues to address.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Boilerplate.Client.Core.csproj (2)

68-72: LGTM! Proper build pipeline for module bundling.

The build sequence is correct:

  1. TypeScript compilation (tsc) generates ES modules in Scripts/
  2. esbuild bundles Scripts/index.js → wwwroot/scripts/app.js
  3. Optional minification for Production builds

This properly implements the module-based architecture described in the PR objectives.


70-70: esbuild is properly declared in devDependencies (version 0.25.11).

The build step correctly references node_modules/.bin/esbuild, which will be available after npm install. No changes needed.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/index.ts (3)

3-10: LGTM! Well-structured entry point imports.

The import organization is excellent:

  • Side-effect imports (lines 3-5) execute initialization code from bswup, theme, and events modules
  • Named imports (lines 6-7) bring in classes for global exposure
  • Conditional compilation (lines 8-10) properly handles the optional Ads feature

This establishes a clear module initialization order as specified in the PR objectives.


12-17: LGTM! Proper global exposure for Blazor interop.

The centralized window exposure pattern:

  • Maintains backward compatibility with C# JSInterop calls
  • Conditionally exposes Ads only when the feature is enabled
  • Provides a single source of truth for global API surface

This addresses the PR objective of defining a clear entry point that coordinates initialization and global exposure.


1-17: Excellent architectural implementation of the entry point module!

This file directly addresses the PR objectives by:

  • Defining a clear entry point that coordinates imports and initialization
  • Establishing explicit module relationships and execution order
  • Centralizing global window exposure for Blazor interop
  • Supporting conditional feature inclusion via template directives

The pattern of "modules export, entry point exposes" is a clean separation of concerns that will improve maintainability.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebInteropApp.ts (3)

1-1: Clarify the //+:cnd:noEmit directive.

The //+:cnd:noEmit comment appears to be a build tool directive but lacks documentation. Consider adding a brief inline comment explaining its purpose for maintainability.

Can you confirm what build tool processes this directive and document it?


4-4: LGTM: Export statement aligns with ES module objectives.

Converting the class to an exported module enables proper import/export usage across TypeScript files, directly addressing the PR objective to replace the single-file outFile approach.


86-86: BitButil type resolution verified—no issues found.

The verification confirms:

  • BitButil is declared once in typings.d.ts (line 9) as declare class BitButil with static webAuthn property
  • Both methods (getCredential, createCredential) are properly typed and match the usage at lines 86 and 102 in WebInteropApp.ts
  • No redeclarations or shadowing detected in the codebase
  • TypeScript compiler will correctly resolve these types globally

The module system changes do not introduce type resolution problems.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/typings.d.ts (2)

1-29: LGTM: Successfully centralizes type declarations per PR objectives.

This new file effectively consolidates TypeScript declarations for external libraries (Blazor, Bit.Butil, Bit.BlazorUI), eliminating the scattered inline declarations mentioned in the PR objectives. The structure is clear and maintainable.


19-27: Well-designed BitThemeOptions interface.

The interface provides comprehensive configuration options with appropriate optional flags, theme variants, and a typed callback. The design supports both automatic (system/persist) and manual theme management effectively.

src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/theme.ts (3)

6-6: LGTM: Optional chaining provides defensive initialization.

The optional chaining operator BitTheme?.init gracefully handles scenarios where the Bit.BlazorUI library may not be loaded, preventing runtime errors. This is good defensive coding practice for modular architecture.


13-14: Good guard against missing CSS variables.

The early return when primaryBgColor is unavailable prevents potential issues with setting an empty theme-color meta tag. This handles edge cases where CSS variables might not be initialized during theme changes.


1-19: LGTM: Clean separation of type declarations from runtime logic.

Successfully moves type declarations to typings.d.ts while preserving the runtime initialization logic. This separation aligns with the PR objective to centralize TypeScript declarations and improves maintainability. The updated comment on line 3 clearly explains the module's purpose.

@msynk msynk changed the title Implement proper module system for bit Boilerplate (#11542) Implement proper js module system for bit Boilerplate (#11542) Nov 1, 2025
@msynk msynk merged commit b7f7f3d into bitfoundation:develop Nov 1, 2025
3 checks passed
@ysmoradi ysmoradi deleted the 11542 branch November 2, 2025 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeScript Configuration Prevents Proper Module System Usage in Boilerplate project template

2 participants