P-2202: P-2209: Examples: Add Angular example for the Formo Web SDK#27
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4191a1404a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "build": { | ||
| "builder": "@ngx-env/builder:application", | ||
| "options": { | ||
| "browser": "src/main.ts", |
There was a problem hiding this comment.
Add the HTML entry point to the build target
The build target never points Angular at the committed src/index.html. For the application builder, the index option is what configures generation/serving of the HTML shell and the schema does not default it, so pnpm start/pnpm build won't use this app's host page and the example won't load correctly at /. Add "index": "src/index.html" alongside browser.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces a new Angular example project demonstrating the integration of the Formo Analytics Web SDK in a non-React environment. The implementation features a dedicated service to wrap the SDK core, an initialization strategy using Angular's app initializer to ensure early event autocapture, and a wallet service utilizing viem for bare EIP-1193 provider interactions. Review feedback focuses on improving type safety and runtime robustness within the wallet service, specifically when handling account addresses returned by the provider.
| if (accounts.length === 0) return; | ||
| const chainIdHex = await provider.request({ method: 'eth_chainId' }); | ||
| this.chainId.set(Number.parseInt(chainIdHex, 16)); | ||
| this.setAddress(accounts[0]); |
There was a problem hiding this comment.
The accounts[0] value is of type string | undefined, but setAddress expects Address | null. Since you've already checked that accounts.length > 0, you can safely cast this to Address to satisfy strict type checking, similar to how it's handled in the accountsChanged listener.
| this.setAddress(accounts[0]); | |
| this.setAddress(accounts[0] as Address); |
| const provider = this.requireProvider(); | ||
|
|
||
| provider.on('accountsChanged', (accounts) => { | ||
| this.setAddress((accounts as Address[])[0] ?? null); |
There was a problem hiding this comment.
The accounts parameter in the accountsChanged event is typed as unknown by default in many EIP-1193 provider definitions. While casting to Address[] is correct for viem, ensure that accounts is indeed an array before accessing the first element to prevent potential runtime errors if a non-compliant provider is used.
| this.setAddress((accounts as Address[])[0] ?? null); | |
| provider.on('accountsChanged', (accounts) => { | |
| const accountList = Array.isArray(accounts) ? (accounts as Address[]) : []; | |
| this.setAddress(accountList[0] ?? null); | |
| }); |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e921a8f24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ### Prerequisites | ||
|
|
||
| - Node.js 20+ |
There was a problem hiding this comment.
Raise the Node prerequisite to Angular 21's minimum
This example depends on Angular 21, whose compatibility table requires Node ^20.19.0 || ^22.12.0 || ^24.0.0 (the lockfile also records that engine range for @angular/cli@21.2.11). With the current Node.js 20+ prerequisite, users on Node 20.0 through 20.18 can follow the README and then hit Angular CLI engine/version failures when running pnpm install, pnpm start, or pnpm build; please document the actual minimum, e.g. Node 20.19+.
Useful? React with 👍 / 👎.
|
updated to work with latest SDK 1.30.1 to reduce unnecessary bloat with react imports |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
should be false positive |
yosriady
left a comment
There was a problem hiding this comment.
please check the failing CI
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Skipping the build step for |
export-1779184131699.mp4
Summary
Adds
with-angular/, an example app demonstrating the Formo Web SDK (@formo/analytics) in Angular — the repo's first non-React example, and the first time the SDK has been exercised on Angular. The SDK's documented integration (FormoAnalyticsProvider/useFormo()) is React-only, and the repo's other wallet examples use wagmi, which is also React-only — so this example wires the framework-agnosticFormoAnalytics.init()core into an Angular service, with a bare EIP-1193 (window.ethereum) wallet andviem. Building it also surfaced two SDK rough edges outside a React/Webpack toolchain.Key Changes
New
with-angular/example:FormoAnalyticsServicewraps the framework-agnosticFormoAnalytics.init()core; initialized viaprovideAppInitializerso the SDK's autocapture wrapswindow.ethereumbefore any wallet interaction is possibleWalletServiceconnects over the bare EIP-1193 provider withviemforsignMessage/sendTransaction— no wagmi; it rehydrates an already-authorized wallet on reload and disconnects viawallet_revokePermissionshistory.pushState, which the SDK already hooksSDK compatibility findings:
Buffer; Angular's esbuild build doesn't polyfill Node globals, so signing threwReferenceError: Buffer is not defined.src/polyfills.tsexposesBufferon the global scope.allowedCommonJsDependenciessilences the resulting build warningsConfig:
@ngx-env/builderreadsNG_APP_FORMO_WRITE_KEYfrom.env, matching the repo's env-file conventionREADME.mdlists the new example under Web FrameworksNeed help on this PR? Tag
@codesmithwith what you need.