Skip to content

Add fido entry point to davinci client package - #744

Open
ancheetah wants to merge 2 commits into
mainfrom
aj-fix-types
Open

Add fido entry point to davinci client package#744
ancheetah wants to merge 2 commits into
mainfrom
aj-fix-types

Conversation

@ancheetah

@ancheetah ancheetah commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

JIRA Ticket

None

Description

This PR adds a new ./fido subpath entry point to @forgerock/davinci-client and ensures the davinci, oidc, and deviceClient factories are explicitly exported from each package's default entry point.

Why

  • Expose fido entry point in addition to fido submodule to be consistent with journey client exports.
  • Explicitly export clients from the default entry points because this is the best we can do without introducing a breaking change. We previously mixed runtime code in the types.ts file due to an issue with API extractor. The types entry point is its own api extractor entry point. API extractor doesn't know about the separate index.ts. So because, for example, the DavinciClient type depends on ReturnType<typeof davinci>, it expects davinci to be exported from types.

Beta

beta of this branch has been published to npm with tag 0.0.0-beta-20260730211401

@changeset-bot

changeset-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 1c03773

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@nx-cloud

nx-cloud Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

View your CI Pipeline Execution ↗ for commit 1c03773

Command Status Duration Result
nx affected -t build lint test typecheck e2e-ci ✅ Succeeded 2m 8s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-30 21:12:33 UTC

@pkg-pr-new

pkg-pr-new Bot commented Jul 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

@forgerock/davinci-client

pnpm add https://pkg.pr.new/@forgerock/davinci-client@744

@forgerock/device-client

pnpm add https://pkg.pr.new/@forgerock/device-client@744

@forgerock/journey-client

pnpm add https://pkg.pr.new/@forgerock/journey-client@744

@forgerock/oidc-client

pnpm add https://pkg.pr.new/@forgerock/oidc-client@744

@forgerock/protect

pnpm add https://pkg.pr.new/@forgerock/protect@744

@forgerock/sdk-types

pnpm add https://pkg.pr.new/@forgerock/sdk-types@744

@forgerock/sdk-utilities

pnpm add https://pkg.pr.new/@forgerock/sdk-utilities@744

@forgerock/iframe-manager

pnpm add https://pkg.pr.new/@forgerock/iframe-manager@744

@forgerock/sdk-logger

pnpm add https://pkg.pr.new/@forgerock/sdk-logger@744

@forgerock/sdk-oidc

pnpm add https://pkg.pr.new/@forgerock/sdk-oidc@744

@forgerock/sdk-request-middleware

pnpm add https://pkg.pr.new/@forgerock/sdk-request-middleware@744

@forgerock/storage

pnpm add https://pkg.pr.new/@forgerock/storage@744

commit: 1c03773

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.98%. Comparing base (eafe277) to head (1c03773).
⚠️ Report is 54 commits behind head on main.

Files with missing lines Patch % Lines
packages/davinci-client/src/index.ts 0.00% 2 Missing ⚠️
packages/davinci-client/src/lib/fido/index.ts 33.33% 2 Missing ⚠️
packages/device-client/src/index.ts 0.00% 1 Missing ⚠️
packages/oidc-client/src/index.ts 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (14.28%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #744       +/-   ##
===========================================
+ Coverage   18.07%   69.98%   +51.91%     
===========================================
  Files         155       60       -95     
  Lines       24398     5148    -19250     
  Branches     1203      939      -264     
===========================================
- Hits         4410     3603      -807     
+ Misses      19988     1545    -18443     
Files with missing lines Coverage Δ
packages/davinci-client/src/lib/fido/fido.types.ts 100.00% <ø> (ø)
packages/device-client/src/index.ts 33.33% <0.00%> (-16.67%) ⬇️
packages/oidc-client/src/index.ts 33.33% <0.00%> (-16.67%) ⬇️
packages/davinci-client/src/index.ts 25.00% <0.00%> (-8.34%) ⬇️
packages/davinci-client/src/lib/fido/index.ts 33.33% <33.33%> (ø)

... and 110 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown
Contributor

Deployed b3d96e2 to https://ForgeRock.github.io/ping-javascript-sdk/pr-744/b3d96e2c1ada0d0bf89c52639c850dd8b7f0db71 branch gh-pages in ForgeRock/ping-javascript-sdk

@github-actions

Copy link
Copy Markdown
Contributor

📦 Bundle Size Analysis

📦 Bundle Size Analysis

🚨 Significant Changes

🔺 @forgerock/davinci-client - 57.1 KB (+2.0 KB, +3.6%)

🆕 New Packages

🆕 @forgerock/journey-client - 92.6 KB (new)
🆕 @forgerock/journey-client - 0.0 KB (new)
🆕 @forgerock/device-client - 10.0 KB (new)
🆕 @forgerock/device-client - 0.0 KB (new)

📊 Minor Changes

📈 @forgerock/oidc-client - 35.4 KB (+0.0 KB)

➖ No Changes

@forgerock/sdk-types - 9.1 KB
@forgerock/sdk-logger - 1.6 KB
@forgerock/iframe-manager - 3.2 KB
@forgerock/sdk-oidc - 5.7 KB
@forgerock/storage - 1.5 KB
@forgerock/sdk-request-middleware - 4.6 KB
@forgerock/sdk-utilities - 18.6 KB
@forgerock/protect - 144.6 KB


14 packages analyzed • Baseline from latest main build

Legend

🆕 New package
🔺 Size increased
🔻 Size decreased
➖ No change

ℹ️ How bundle sizes are calculated
  • Current Size: Total gzipped size of all files in the package's dist directory
  • Baseline: Comparison against the latest build from the main branch
  • Files included: All build outputs except source maps and TypeScript build cache
  • Exclusions: .map, .tsbuildinfo, and .d.ts.map files

🔄 Updated automatically on each push to this PR

@ancheetah
ancheetah marked this pull request as ready for review July 30, 2026 20:53
@ancheetah
ancheetah requested a review from ryanbas21 July 30, 2026 20:53
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

The package configuration declares CommonJS mode and adds the ./fido export. DaVinci FIDO implementation and types are exposed through new entrypoints and documented in an API report. DaVinci, Device, and OIDC entrypoints now directly re-export their client implementations.

Public package exports

Layer / File(s) Summary
Package export wiring
package.json, packages/davinci-client/package.json
CommonJS mode is declared, script entries are reordered, and the ./fido package export maps to its built entrypoint.
FIDO public API surface
packages/davinci-client/src/lib/fido/*, packages/davinci-client/api-report/*
The FIDO entrypoint re-exports the implementation and public types, while the API report documents the FIDO factory, client methods, input/options types, WebAuthn values, and error codes.
Client entrypoint re-exports
packages/davinci-client/src/index.ts, packages/device-client/src/index.ts, packages/oidc-client/src/index.ts
DaVinci, Device, and OIDC entrypoints directly re-export their client implementations and update copyright headers.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: ryanbas21

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the main change: adding a FIDO entry point to the davinci client package.
Description check ✅ Passed The description includes the required JIRA Ticket and Description sections and explains the rationale, though it omits an explicit changeset note.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aj-fix-types

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/davinci-client/src/index.ts (1)

8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add package-level tests for all new client exports.

The new public re-exports are uncovered, causing the patch to fail the 40% Codecov target.

  • packages/davinci-client/src/index.ts#L8-L9: test root imports for davinci and fido.
  • packages/device-client/src/index.ts#L7: test the root deviceClient export.
  • packages/oidc-client/src/index.ts#L7: test the root oidc export.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/davinci-client/src/index.ts` around lines 8 - 9, Add package-level
tests covering the root exports: import and exercise davinci and fido from
packages/davinci-client/src/index.ts (lines 8-9), deviceClient from
packages/device-client/src/index.ts (line 7), and oidc from
packages/oidc-client/src/index.ts (line 7). Follow existing package test
conventions and ensure each public export is verified sufficiently to meet the
Codecov threshold.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/davinci-client/src/lib/fido/index.ts`:
- Around line 8-10: Add focused coverage for the FIDO barrel in index.ts by
importing `@forgerock/davinci-client/fido`, exercising the exported fido path, and
adding a compile-time import that validates the re-exported types from
fido.types.js.

---

Nitpick comments:
In `@packages/davinci-client/src/index.ts`:
- Around line 8-9: Add package-level tests covering the root exports: import and
exercise davinci and fido from packages/davinci-client/src/index.ts (lines 8-9),
deviceClient from packages/device-client/src/index.ts (line 7), and oidc from
packages/oidc-client/src/index.ts (line 7). Follow existing package test
conventions and ensure each public export is verified sufficiently to meet the
Codecov threshold.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: de0f2a3e-7a90-483f-b255-4b5d444eccf0

📥 Commits

Reviewing files that changed from the base of the PR and between d65f42a and 1c03773.

📒 Files selected for processing (8)
  • package.json
  • packages/davinci-client/api-report/davinci-client.fido.api.md
  • packages/davinci-client/package.json
  • packages/davinci-client/src/index.ts
  • packages/davinci-client/src/lib/fido/fido.types.ts
  • packages/davinci-client/src/lib/fido/index.ts
  • packages/device-client/src/index.ts
  • packages/oidc-client/src/index.ts

Comment on lines +8 to +10
// Re-export FIDO module and types for package.json `./fido` entry point
export { fido } from './fido.js';
export * from './fido.types.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Cover the new FIDO barrel before merging.

Codecov reports this changed file as uncovered, leaving patch coverage at 14.28571% against the 40% gate. Add a focused test that imports @forgerock/davinci-client/fido and exercises the exported fido path, with a compile-time import for the type re-exports.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/davinci-client/src/lib/fido/index.ts` around lines 8 - 10, Add
focused coverage for the FIDO barrel in index.ts by importing
`@forgerock/davinci-client/fido`, exercising the exported fido path, and adding a
compile-time import that validates the re-exported types from fido.types.js.

@ancheetah

Copy link
Copy Markdown
Collaborator Author

beta of this branch has been published to npm with tag 0.0.0-beta-20260730211401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants