Skip to content

add Angular adapter package#154

Open
adriandarian wants to merge 4 commits into
spaansba:mainfrom
adriandarian:main
Open

add Angular adapter package#154
adriandarian wants to merge 4 commits into
spaansba:mainfrom
adriandarian:main

Conversation

@adriandarian

Copy link
Copy Markdown

Summary

Adds a first-party Angular adapter package alongside the existing React and Vue bindings.

The new @foresightjs/angular package provides:

  • ForesightDirective, a standalone [fsForesight] directive for HTML and SVG elements
  • ForesightService, an injectable manual registration and event-subscription API
  • ForesightComponent, a standalone host-wrapper component
  • injectForesightEvent, an Angular injection-context helper for manager events
  • Re-exported core ForesightJS types to match the React and Vue package ergonomics
  • Vitest coverage for registration, unregistering, option patching, latest callback forwarding, signal state updates, SVG registration, and event cleanup

Flow

flowchart LR
  A[Angular template] --> B[ForesightDirective or ForesightComponent]
  B --> C[ForesightService]
  C --> D[ForesightManager]
  D --> E[Core prediction state]
  E --> F[Angular signal state]
  D --> G[Callback or event listener]
Loading

Validation

  • pnpm --filter @foresightjs/angular lint
  • pnpm --filter @foresightjs/angular test:run
  • pnpm --filter @foresightjs/angular build
  • pnpm build

All passed locally.

Notes

The Angular package is compiled with ng-packagr in Angular partial-compilation mode, which is the expected publishing format for Angular libraries. Declared peer support is Angular 17 through Angular 22 because the adapter uses standalone APIs and signals.

@adriandarian adriandarian marked this pull request as ready for review June 25, 2026 08:32
@adriandarian adriandarian changed the title [codex] add Angular adapter package add Angular adapter package Jun 25, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

The Angular adapter is well-structured and consistent with the existing React/Vue packages, but a couple of correctness and packaging issues should be addressed before merging.

Reviewed changes — adds a first-party Angular adapter package (@foresightjs/angular) with a directive, host-wrapper component, service, signal helper, and tests alongside ng-packagr build wiring.

  • Adds ForesightDirective, ForesightComponent, ForesightService, and injectForesightEvent
  • Wires ng-packagr partial compilation, Vitest with jsdom, and workspace path aliases
  • Mirrors the React/Vue public API shape with core re-exports and ForesightComponentOptions

⚠️ User callbacks run outside the Angular zone

ForesightService wraps state updates and manager events in this.zone.run, but the user-supplied callback is invoked bare inside ForesightManager. If a consumer mutates Angular state in their callback, change detection will not run in a Zone.js app.

Technical details
# User callbacks run outside the Angular zone

## Affected sites
- `packages/foresightjs-angular/src/services/ForesightService.ts:23` — callback wrapper does not re-enter Angular zone

## Required outcome
- User callbacks registered with `ForesightService.register()` must be executed inside `NgZone.run` so Angular change detection is triggered.

## Suggested approach
Change the callback wrapper to:
```ts
const callback = (s: ForesightElementState) => this.zone.run(() => optionsRef.callback(s))
```

Optionally wrap the synchronous `state.set` calls at lines 37 and 64 in `zone.run` as well for consistency.

⚠️ tslib is only a dev dependency

tsconfig.json sets importHelpers: true, so compiled FESM output imports tslib. With tslib only in devDependencies, consumers using strict package managers may not resolve it at runtime.

Technical details
# `tslib` dependency placement

## Affected sites
- `packages/foresightjs-angular/package.json:66``tslib` under `devDependencies`
- `packages/foresightjs-angular/tsconfig.json:10``importHelpers: true`

## Required outcome
- Add `tslib` as a peer dependency (or a runtime dependency) so installed consumers can resolve it. Typical Angular libraries use a peer dependency like `"tslib": "^2.8.1"`.

⚠️ build and build:prod produce identical artifacts

The two scripts are the same command, unlike React and Vue where build emits sourcemaps and build:prod strips them.

Technical details
# `build` vs `build:prod`

## Affected sites
- `packages/foresightjs-angular/package.json:8-9` — both scripts identical

## Required outcome
- Production builds should not ship sourcemaps.

## Suggested approach
Create a `tsconfig.lib.prod.json` that disables `sourceMap`/`inlineSources`, then change `build:prod` to use it:
```json
"build:prod": "ng-packagr -p ng-package.json -c tsconfig.lib.prod.json"
```

ℹ️ Nitpicks

  • package.json:67 declares typescript: "^6.0.2" but TypeScript 6 has not been released; Angular 22 currently expects TypeScript 5.x.
  • LICENSE:3 uses 2024 while the other packages use 2025.
  • README.md:10 links to /docs/angular but React/Vue docs use /docs/<framework>/installation.
  • No tests currently cover ForesightComponent, injectForesightEvent, SSR guards, or Zone re-entry behavior.
  • Consider removing the redundant *.d.ts wildcard from files to match @foresightjs/react and @foresightjs/vue.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏

Comment thread packages/foresightjs-angular/src/services/ForesightService.ts Outdated
Comment thread packages/foresightjs-angular/package.json
Comment thread packages/foresightjs-angular/package.json Outdated
Comment thread packages/foresightjs-angular/package.json
Comment thread packages/foresightjs-angular/src/helpers/injectForesightEvent.ts
@adriandarian

Copy link
Copy Markdown
Author

Addressed the review feedback in 15b01e6.

Changes made:

  • Wrapped registered user callbacks in NgZone.run, and also run initial state sync plus unregister reset through the Angular zone.
  • Added tests for callback zone re-entry and state reset zone re-entry.
  • Moved tslib to a runtime dependencies entry so strict package managers can resolve Angular helper imports. I used dependencies instead of peerDependencies because ng-packagr 22 explicitly warns that tslib should no longer be a peer dependency and writes it as a dependency in generated packages.
  • Split build:prod from build with tsconfig.lib.prod.json and a post-build sourcemap removal script.
  • Removed the plain getter-function event type input from injectForesightEvent; dynamic event types/listeners should be Angular signals, while plain values are static.
  • Cleaned up README docs URL, LICENSE year, and the redundant *.d.ts package file entry.

Validation:

  • pnpm --filter @foresightjs/angular lint
  • pnpm --filter @foresightjs/angular test:run — 15 tests passing
  • pnpm --filter @foresightjs/angular build
  • pnpm --filter @foresightjs/angular build:prod
  • verified production dist output has no .map files immediately after build:prod
  • pnpm build

All five review threads have been replied to and resolved.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — minor cleanup suggestions inline.

Reviewed changes — author response at 15b01e6 addresses the prior review's must-fix items: Zone.js re-entry for callbacks and state updates, moving tslib to runtime dependencies, and splitting build:prod to use a sourcemap-free production tsconfig.

  • Wrapped ForesightService callbacks, state updates, and manager-event listeners in NgZone.run
  • Added unit tests verifying callback and state-reset Zone re-entry
  • Moved tslib from devDependencies to dependencies
  • Added tsconfig.lib.prod.json and a remove-sourcemaps.mjs cleanup script for production builds
  • Fixed LICENSE year, README docs link, and removed redundant *.d.ts from files
  • Restricted injectForesightEvent dynamic inputs to Angular Signals and documented static vs. dynamic behavior

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏

Comment thread packages/foresightjs-angular/package.json Outdated
Comment thread packages/foresightjs-angular/scripts/remove-sourcemaps.mjs

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — minor suggestions inline.

Reviewed changes — added the Angular documentation site tree, sidebar entry, framework selector, beta banner detection, and README/CONTRIBUTING updates for the new @foresightjs/angular package.

  • Added the full Angular docs tree (directive, foresight-service, foresight-component, injectForesightEvent, quick-start, installation, etc.)
  • Wired angularSidebar and framework-switching support across the docs site
  • Updated READMEs, CONTRIBUTING, and the core what-is/migrating cross-references to list Angular alongside React and Vue

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏


/**
* Renders the early-release notice on every page of the React and Vue
* Renders the early-release notice on every page of the framework package

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment now reads "framework package framework trees" — one of the two nouns should be removed to avoid the doubled-up phrase.

author: Bart Spaans
---

import MigratingCore from "../_partials/_migrating-core.mdx";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The MDX import ends with a semicolon, while the other new Angular doc pages omit it. Aligning with the no-semicolon style keeps the Angular docs consistent.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — follow-up commit f12570d resolves the two remaining cleanup items from the prior review.

  • Removed the duplicate tslib entry from devDependencies
  • Guarded remove-sourcemaps.mjs against a missing dist directory with existsSync

Pullfrog  | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏

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.

1 participant