feat: add-recognize-proxy#580
Conversation
|
📝 WalkthroughWalkthroughAdds a new ChangesRecognize + e2e app
Sequence Diagram(s)sequenceDiagram
participant Client as Client Code
participant Recognize as recognize()
participant SDK as recognize-sdk (dynamic import)
participant DOM as Browser DOM
participant Element as kl-auth / kl-enroll
Client->>Recognize: call recognize(config)
Recognize-->>Client: return client (subscribe/init/dispose)
Client->>Recognize: client.init({mode, ...})
Recognize->>SDK: dynamic import('./recognize-sdk')
SDK-->>Recognize: SDK module (elements) loaded
Recognize->>DOM: querySelector('kl-auth' or 'kl-enroll')
DOM-->>Recognize: element or null
alt element found or attach mode
Recognize->>Element: set username, publicKey, disableSteps, other props
Recognize->>Element: attach event listeners (step-change, finished, error, frame-results, ws-open/close)
Element-->>Recognize: emits events -> Recognize dispatches to observers
else create-and-append
Recognize->>DOM: createElement('kl-auth'|'kl-enroll') and append to container
end
Client->>Recognize: client.dispose()
Recognize->>Element: abort listeners, remove element, clear observers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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. Comment |
|
View your CI Pipeline Execution ↗ for commit 39ee8bf
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/recognize-app/eslint.config.mjs`:
- Around line 15-20: The glob ignore patterns in eslint.config.mjs (entries like
'*.html', '*.svg', '*.css', '*.json', '*.d.ts') are non-recursive and only match
root-level files; update each asset pattern to be recursive (e.g., change
'*.html' -> '**/*.html', '*.css' -> '**/*.css', '*.svg' -> '**/*.svg', '*.json'
-> '**/*.json', '*.d.ts' -> '**/*.d.ts') so nested assets (like src/index.html
or src/styles/site.css) are ignored by ESLint; modify the array of ignore globs
in the file accordingly to use the double-star recursive form for all asset
patterns.
In `@e2e/recognize-app/vite.config.ts`:
- Around line 8-10: The Vite config sets root to src but publicDir points to
__dirname + '/public', so static assets under the Vite root (src/public) won't
be served; update publicDir to reference the Vite root's public folder (e.g.,
derive it from the configured root or point it to src/public) so that with root
= __dirname + '/src' the publicDir resolves to the corresponding public
directory; adjust the publicDir setting in vite.config.ts accordingly
(referencing the root/public relationship rather than the project root).
In `@packages/recognize/package.json`:
- Around line 17-22: Add the missing runtime/type dependencies referenced by the
recognize-sdk imports to package.json: declare `@aracna/web-components`,
`@aracna/core`, `@keyless/sdk-web`, lit, motion, and `@ua-parser-js/pro-enterprise`
under either "dependencies" or "peerDependencies" (choose peerDependencies if
these should be provided by consumers, otherwise dependencies to bundle them),
ensuring the package names match the imports used by the SDK so consumers have
the packages available at runtime and for type checking.
In `@packages/recognize/src/lib/recognize-sdk/index.d.ts`:
- Around line 92-100: The autogenerated SDK declaration file contains many empty
interfaces and CustomEvent<{}> types (e.g., KeylessAuthElementAttributes,
KeylessAuthElementEventMap, KeylessBeginStreamEvent) which trigger
`@typescript-eslint` rules; either exclude these files from ESLint by adding a
glob like recognize-sdk/**/*.d.ts to the lint ignore/overrides, or regenerate
the SDK with types that avoid empty-interface and empty-object types so
declarations like KeylessAuthElementAttributes and KeylessBeginStreamEvent no
longer use {} or empty interfaces; pick one approach and update project lint
config or the generator output accordingly.
In `@packages/recognize/src/lib/recognize.ts`:
- Around line 20-32: The variable recognizeApiInitialized is declared and set
but never used; remove its declaration and the assignment to it inside the start
function (references: recognizeApiInitialized, start) so the module no longer
contains unused state—keep recognizeElement and the SDK import/logic intact.
- Around line 39-44: Replace the loose-typed `element` lookup with a const and
an explicit DOM generic/cast so TypeScript knows the query returns our custom
element union: call `document.querySelector` with the generic type
`KeylessAuthElement | KeylessEnrollElement` (or cast the result) into a `const
el` and then assign it to `recognizeElement` after the null check; update the
thrown Error to remain unchanged but ensure you reference the previously named
symbols `element`/`recognizeElement`/`KeylessAuthElement`/`KeylessEnrollElement`
so the assignment is stable under TypeScript DOM typings.
In `@packages/recognize/src/lib/recognize.types.ts`:
- Around line 34-37: The JSDoc incorrectly marks webSocketUrl as optional while
the interface property webSocketUrl is required; update the comment for the
webSocketUrl property in recognize.types.ts to remove the optional brackets and
reflect it's required (e.g., change "@property {string} [webSocketUrl]" to
"@property {string} webSocketUrl") and ensure the description remains accurate
for the PingOne Recognize SDK connection URL.
In `@packages/recognize/vitest.config.mts`:
- Around line 1-4: The config currently uses the CJS-only __dirname and a .mts
extension that won't be discovered by the workspace pattern; rename
vitest.config.mts → vitest.config.ts and update the root value to be ESM-safe by
deriving it from import.meta.url (e.g., convert import.meta.url to a file path
and use its directory) so the config runs under ESM and matches the existing
workspace discovery pattern '**/*/vitest.config.ts'.
🪄 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
Run ID: c70823ca-c533-496d-b706-b0a3697959e1
⛔ Files ignored due to path filters (3)
packages/recognize/src/lib/recognize-sdk/pthreads/wasm.wasmis excluded by!**/*.wasmpackages/recognize/src/lib/recognize-sdk/wasm.wasmis excluded by!**/*.wasmpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
e2e/recognize-app/eslint.config.mjse2e/recognize-app/package.jsone2e/recognize-app/src/index.htmle2e/recognize-app/src/index.tse2e/recognize-app/src/styles.csse2e/recognize-app/tsconfig.app.jsone2e/recognize-app/tsconfig.jsone2e/recognize-app/vite.config.tspackages/recognize/README.mdpackages/recognize/eslint.config.mjspackages/recognize/package.jsonpackages/recognize/src/index.tspackages/recognize/src/lib/recognize-sdk/index.d.tspackages/recognize/src/lib/recognize-sdk/index.jspackages/recognize/src/lib/recognize-sdk/pthreads/wasm.jspackages/recognize/src/lib/recognize-sdk/wasm.datapackages/recognize/src/lib/recognize-sdk/wasm.jspackages/recognize/src/lib/recognize.spec.tspackages/recognize/src/lib/recognize.tspackages/recognize/src/lib/recognize.types.tspackages/recognize/tsconfig.jsonpackages/recognize/tsconfig.lib.jsonpackages/recognize/tsconfig.spec.jsonpackages/recognize/vitest.config.mtstsconfig.json
| '*.html', | ||
| '*.svg', | ||
| '*.css', | ||
| 'public', | ||
| '*.json', | ||
| '*.d.ts', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List nested asset/config files under the recognize E2E app that root-only ignore globs do not cover.
python - <<'PY'
from pathlib import Path
root = Path("e2e/recognize-app")
suffixes = (".html", ".svg", ".css", ".json", ".d.ts")
for path in root.rglob("*"):
if path.is_file() and path.name.endswith(suffixes):
rel = path.relative_to(root).as_posix()
if "/" in rel:
print(rel)
PYRepository: ForgeRock/ping-javascript-sdk
Length of output: 102
🏁 Script executed:
cd e2e/recognize-app && head -25 eslint.config.mjsRepository: ForgeRock/ping-javascript-sdk
Length of output: 438
🏁 Script executed:
# Check if there are other nested files beyond what was already found
find e2e/recognize-app -type f \( -name "*.html" -o -name "*.svg" -o -name "*.css" -o -name "*.json" -o -name "*.d.ts" \) | head -20Repository: ForgeRock/ping-javascript-sdk
Length of output: 237
🌐 Web query:
ESLint flat config ignore patterns glob semantics documentation
💡 Result:
In ESLint flat config (eslint.config.js, default since v9), ignore patterns use minimatch glob syntax. They are specified in the ignores array within config objects. Global ignores (applied project-wide, skipping file traversal): Use a standalone object with only ignores or the globalIgnores helper from eslint/config. Example: import { globalIgnores } from "eslint/config"; export default [ globalIgnores(["/node_modules/", "dist/", ".git/"]), // other configs ]; Local ignores (apply only to that config object): Use ignores alongside files, rules, etc. Example: { files: ["/.js"], ignores: ["/.config.js"], rules: { /* ... / } } Glob semantics (minimatch): - recurses directories (e.g., /node_modules/ ignores all node_modules). - * matches non-/ chars. - ? single non-/ char. - dir/ matches only that dir (relative to config file location). - dir/ ignores dir + contents (no unignore inside). - dir// ignores contents but allows unignoring specific files. - !pattern negates (unignores). - Patterns ending / match dirs; others match files/dirs. - Defaults: ["/node_modules/", ".git/"]. - Resolution: Relative to eslint.config.js for globalIgnores; cwd for CLI --ignore-pattern. Unlike .eslintignore (gitignore-style), flat config uses strict minimatch (e.g., dist ignores only top-level dist, use /dist/ for recursive). Primary source: Official ESLint docs (https://eslint.org/docs/latest/use/configure/ignore).
Citations:
- 1: https://eslint.org/docs/latest/use/configure/ignore
- 2: https://eslint.org/blog/2025/03/flat-config-extends-define-config-global-ignores/
- 3: https://eslint.org/docs/user-guide/configuring/configuration-files-new
Make asset ignore globs recursive.
Patterns like *.html and *.css match only root-level files in ESLint's flat config (using strict minimatch). Nested files such as src/index.html and src/styles.css will not be ignored, allowing the linter to process them despite being asset files.
🛠️ Proposed fix
- '*.html',
- '*.svg',
- '*.css',
+ '**/*.html',
+ '**/*.svg',
+ '**/*.css',
'public',
- '*.json',
- '*.d.ts',
+ '**/*.json',
+ '**/*.d.ts',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| '*.html', | |
| '*.svg', | |
| '*.css', | |
| 'public', | |
| '*.json', | |
| '*.d.ts', | |
| '**/*.html', | |
| '**/*.svg', | |
| '**/*.css', | |
| 'public', | |
| '**/*.json', | |
| '**/*.d.ts', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/recognize-app/eslint.config.mjs` around lines 15 - 20, The glob ignore
patterns in eslint.config.mjs (entries like '*.html', '*.svg', '*.css',
'*.json', '*.d.ts') are non-recursive and only match root-level files; update
each asset pattern to be recursive (e.g., change '*.html' -> '**/*.html',
'*.css' -> '**/*.css', '*.svg' -> '**/*.svg', '*.json' -> '**/*.json', '*.d.ts'
-> '**/*.d.ts') so nested assets (like src/index.html or src/styles/site.css)
are ignored by ESLint; modify the array of ignore globs in the file accordingly
to use the double-star recursive form for all asset patterns.
| root: __dirname + '/src', | ||
| cacheDir: '../../node_modules/.vite/e2e/recognize-app', | ||
| publicDir: __dirname + '/public', |
There was a problem hiding this comment.
Point publicDir at the Vite root’s public folder.
With root set to src, Line 10 currently serves e2e/recognize-app/public, not e2e/recognize-app/src/public. If the app’s static assets are under src/public, they won’t be copied or served.
🛠️ Proposed fix
- publicDir: __dirname + '/public',
+ publicDir: resolve(__dirname, 'src/public'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/recognize-app/vite.config.ts` around lines 8 - 10, The Vite config sets
root to src but publicDir points to __dirname + '/public', so static assets
under the Vite root (src/public) won't be served; update publicDir to reference
the Vite root's public folder (e.g., derive it from the configured root or point
it to src/public) so that with root = __dirname + '/src' the publicDir resolves
to the corresponding public directory; adjust the publicDir setting in
vite.config.ts accordingly (referencing the root/public relationship rather than
the project root).
| "dependencies": { | ||
| "tslib": "^2.3.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@aracna/web-components": "^1.1.16" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
manifest="$(fd -a '^package\.json$' packages/recognize | head -n 1)"
sdk_dts="$(fd -a '^index\.d\.ts$' packages/recognize/src/lib/recognize-sdk | head -n 1)"
echo "Package manifest: $manifest"
jq '{dependencies, peerDependencies, devDependencies}' "$manifest"
echo
echo "External imports used by recognize-sdk declaration:"
rg -n "from '(@[^']+|[^./][^']*)'" "$sdk_dts" -o --replace '$1' | sort -uRepository: ForgeRock/ping-javascript-sdk
Length of output: 1419
Add missing SDK dependencies to package.json.
The copied recognize-sdk declarations import several packages (@aracna/web-components, @aracna/core, @keyless/sdk-web, lit, motion, @ua-parser-js/pro-enterprise) that are not listed in dependencies or peerDependencies. Consumers will lack these packages at runtime and during type checking. Declare them as dependencies or peer dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/recognize/package.json` around lines 17 - 22, Add the missing
runtime/type dependencies referenced by the recognize-sdk imports to
package.json: declare `@aracna/web-components`, `@aracna/core`, `@keyless/sdk-web`,
lit, motion, and `@ua-parser-js/pro-enterprise` under either "dependencies" or
"peerDependencies" (choose peerDependencies if these should be provided by
consumers, otherwise dependencies to bundle them), ensuring the package names
match the imports used by the SDK so consumers have the packages available at
runtime and for type checking.
| export declare interface KeylessAuthElementAttributes extends RootElementAttributes { | ||
| } | ||
|
|
||
| /** @public */ | ||
| export declare interface KeylessAuthElementEventMap extends RootElementEventMap { | ||
| } | ||
|
|
||
| /** @public */ | ||
| export declare class KeylessBeginStreamEvent extends CustomEvent<{}> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sdk_dts="$(fd -a '^index\.d\.ts$' packages/recognize/src/lib/recognize-sdk | head -n 1)"
echo "Generated declaration file:"
echo "$sdk_dts"
echo
echo "Empty extension interfaces:"
rg -n "export declare interface .* extends .* \\{\\s*\\}" "$sdk_dts"
echo
echo "CustomEvent<{}> declarations:"
rg -n "CustomEvent<\\{\\}>" "$sdk_dts"
echo
echo "Potential ESLint ignore/override files:"
fd -a '(^eslint\.config\.(js|mjs|cjs)$|\.eslintignore$|\.eslintrc.*)'Repository: ForgeRock/ping-javascript-sdk
Length of output: 200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sdk_dts="packages/recognize/src/lib/recognize-sdk/index.d.ts"
echo "=== File content (lines 92-100) ==="
sed -n '92,100p' "$sdk_dts"
echo
echo "=== File content (lines 129-144) ==="
sed -n '129,144p' "$sdk_dts"
echo
echo "=== Check for empty interfaces (with multiline) ==="
rg -U "interface\s+\w+.*\{\s*\}" "$sdk_dts" | head -20
echo
echo "=== Check CustomEvent patterns ==="
rg "CustomEvent" "$sdk_dts" | head -10
echo
echo "=== ESLint configuration files ==="
find . -maxdepth 2 -type f \( -name "eslint.config.*" -o -name ".eslintrc*" -o -name ".eslintignore" \) -exec echo "Found: {}" \;Repository: ForgeRock/ping-javascript-sdk
Length of output: 2967
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ESLint configuration (eslint.config.mjs) ==="
cat ./eslint.config.mjs | head -100
echo
echo "=== Search for recognize-sdk exclusions in eslint.config.mjs ==="
grep -n "recognize" ./eslint.config.mjs || echo "No 'recognize' found in ESLint config"
echo
echo "=== Check package.json in recognize for comments ==="
grep -A 5 -B 5 '"recognize-sdk"' packages/recognize/package.json || echo "No recognize-sdk in recognize/package.json"
echo
echo "=== Count total empty interfaces and CustomEvent<{}> ==="
echo "Empty interfaces: $(rg -U "interface\s+\w+.*\{\s*\}" packages/recognize/src/lib/recognize-sdk/index.d.ts | wc -l)"
echo "CustomEvent<{}>: $(rg "CustomEvent<\{\}>" packages/recognize/src/lib/recognize-sdk/index.d.ts | wc -l)"
echo
echo "=== Check file header/comments for generation info ==="
head -30 packages/recognize/src/lib/recognize-sdk/index.d.tsRepository: ForgeRock/ping-javascript-sdk
Length of output: 5441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full ESLint configuration ==="
wc -l ./eslint.config.mjs
echo
echo "=== Search for .d.ts overrides or rules in eslint.config ==="
grep -n "\.d\.ts\|\.ts\|overrides\|no-empty-interface\|no-empty-object-type" ./eslint.config.mjs || echo "No specific patterns found"
echo
echo "=== Get the rules section in detail ==="
sed -n '/^ {$/,/^ },$/p' ./eslint.config.mjs | tail -50
echo
echo "=== Check TypeScript ESLint recommended config (what rules are enabled) ==="
rg "@typescript-eslint/recommended" . -A 5 | head -20
echo
echo "=== Check if recognize package has its own eslint config ==="
ls -la packages/recognize/ | grep -i eslint || echo "No eslint config in recognize package"
echo
echo "=== Check if .d.ts files trigger linting errors by searching for lint ignore comments ==="
head -100 packages/recognize/src/lib/recognize-sdk/index.d.ts | grep -i "eslint\|lint\|ignore" || echo "No lint directives found"Repository: ForgeRock/ping-javascript-sdk
Length of output: 2318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== recognize package eslint.config.mjs ==="
cat packages/recognize/eslint.config.mjs
echo
echo "=== Check what `@typescript-eslint/recommended` includes ==="
npm list `@typescript-eslint/eslint-plugin` 2>/dev/null | head -5 || echo "ESLint not available for direct rule inspection"
echo
echo "=== Search for any overrides of recognize-sdk in local config ==="
grep -r "recognize-sdk\|\.d\.ts" packages/recognize/eslint.config.mjs || echo "No recognize-sdk overrides found"
echo
echo "=== Try to run eslint on the file to see actual errors ==="
cd packages/recognize
if command -v npx &> /dev/null; then
npx eslint src/lib/recognize-sdk/index.d.ts 2>&1 | head -50 || echo "ESLint run failed or no errors"
else
echo "npx not available in sandbox"
fiRepository: ForgeRock/ping-javascript-sdk
Length of output: 27357
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if no-empty-interface rule is configured anywhere ==="
rg "no-empty-interface\|no-empty-object-type" . --type json --type ts --type js || echo "No explicit rule configuration found"
echo
echo "=== Check TypeScript eslint recommended preset details ==="
npm info `@typescript-eslint/eslint-plugin`@8.46.3 2>/dev/null | grep -A 20 "recommended" || echo "npm info not available"
echo
echo "=== Verify line endings in the recognize-sdk file ==="
file packages/recognize/src/lib/recognize-sdk/index.d.ts
echo
echo "=== Check git config for line endings ==="
cd packages/recognize && git config core.safecrlf || echo "No safecrlf setting"Repository: ForgeRock/ping-javascript-sdk
Length of output: 374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check `@typescript-eslint/recommended` config via node ==="
node <<'EOF'
try {
const plugin = require('@typescript-eslint/eslint-plugin');
const configs = plugin.configs;
if (configs && configs.recommended) {
const ruleNames = Object.keys(configs.recommended.rules || {});
const emptyInterfaceRules = ruleNames.filter(r => r.includes('empty'));
console.log('Rules with "empty" in name:', emptyInterfaceRules);
if (configs.recommended.rules['@typescript-eslint/no-empty-interface']) {
console.log('@typescript-eslint/no-empty-interface:', configs.recommended.rules['@typescript-eslint/no-empty-interface']);
}
}
} catch(e) {
console.log('Could not load plugin:', e.message);
}
EOF
echo
echo "=== Run eslint on the file with more context (look for actual TypeScript errors) ==="
cd packages/recognize
npx eslint src/lib/recognize-sdk/index.d.ts --format json 2>&1 | python3 <<'PYEOF'
import json
import sys
try:
data = json.loads(sys.stdin.read())
if isinstance(data, list) and len(data) > 0:
messages = data[0].get('messages', [])
# Filter to non-prettier errors
non_prettier = [m for m in messages if 'prettier' not in m.get('ruleId', '').lower()]
print(f"Total messages: {len(messages)}")
print(f"Non-prettier errors: {len(non_prettier)}")
if non_prettier:
print("\nNon-prettier errors:")
for msg in non_prettier[:10]:
print(f" {msg.get('ruleId')}: {msg.get('message')} (line {msg.get('line')})")
else:
print("No TypeScript-ESLint rule violations found")
except:
print("Could not parse JSON output")
PYEOF
echo
echo "=== Check if recognize has .d.ts exclusion rules ==="
grep -n "d\.ts\|declaration" packages/recognize/eslint.config.mjs || echo "No .d.ts handling in recognize config"Repository: ForgeRock/ping-javascript-sdk
Length of output: 321
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Run eslint and capture full output (redirect stderr) ==="
cd packages/recognize
npx eslint src/lib/recognize-sdk/index.d.ts 2>&1 | grep -v "Delete.*␍\|Replace.*␍" | head -50
echo
echo "=== Count total errors by type ==="
npx eslint src/lib/recognize-sdk/index.d.ts 2>&1 | grep -oE "^[[:space:]]*[0-9]+:[0-9]+[[:space:]]+error" | wc -l
echo
echo "=== Check node_modules for `@typescript-eslint` rule definition ==="
find node_modules -name "*.js" -path "*@typescript-eslint*no-empty*" 2>/dev/null | head -5
echo
echo "=== Get the actual rule definition ==="
if [ -f "node_modules/@typescript-eslint/eslint-plugin/dist/rules/no-empty-object-type.js" ]; then
head -100 node_modules/@typescript-eslint/eslint-plugin/dist/rules/no-empty-object-type.js | tail -50
elif [ -f "node_modules/@typescript-eslint/eslint-plugin/dist/rules/no-empty-interface.js" ]; then
head -50 node_modules/@typescript-eslint/eslint-plugin/dist/rules/no-empty-interface.js
else
echo "Rule files not found in expected locations"
fi
echo
echo "=== Summary: Check if the file actually causes ESLint failures ==="
npx eslint src/lib/recognize-sdk/index.d.ts 2>&1 | tail -3Repository: ForgeRock/ping-javascript-sdk
Length of output: 18419
Exclude this generated SDK declaration file from linting or regenerate with compatible types.
This vendored .d.ts currently fails linting with multiple @typescript-eslint/no-empty-interface, @typescript-eslint/no-empty-object-type, and empty-object-type violations in CustomEvent<{}> declarations. Rather than manually patching dozens of declarations, exclude recognize-sdk/**/*.d.ts from lint (via .eslintignore or config overrides) or regenerate the SDK types with lint-compatible output.
Violations occur at: lines 92, 96, 100, 129, 140, 144, 196, 244, 280, 284, 289, 356, 416, 447, and others.
🧰 Tools
🪛 ESLint
[error] 92-92: An interface declaring no members is equivalent to its supertype.
(@typescript-eslint/no-empty-object-type)
[error] 92-92: An interface declaring no members is equivalent to its supertype.
(@typescript-eslint/no-empty-interface)
[error] 96-96: An interface declaring no members is equivalent to its supertype.
(@typescript-eslint/no-empty-object-type)
[error] 96-96: An interface declaring no members is equivalent to its supertype.
(@typescript-eslint/no-empty-interface)
[error] 100-100: The {} ("empty object") type allows any non-nullish value, including literals like 0 and "".
- If that's what you want, disable this lint rule with an inline comment or configure the 'allowObjectTypes' rule option.
- If you want a type meaning "any object", you probably want
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead.
(@typescript-eslint/no-empty-object-type)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/recognize/src/lib/recognize-sdk/index.d.ts` around lines 92 - 100,
The autogenerated SDK declaration file contains many empty interfaces and
CustomEvent<{}> types (e.g., KeylessAuthElementAttributes,
KeylessAuthElementEventMap, KeylessBeginStreamEvent) which trigger
`@typescript-eslint` rules; either exclude these files from ESLint by adding a
glob like recognize-sdk/**/*.d.ts to the lint ignore/overrides, or regenerate
the SDK with types that avoid empty-interface and empty-object types so
declarations like KeylessAuthElementAttributes and KeylessBeginStreamEvent no
longer use {} or empty interfaces; pick one approach and update project lint
config or the generator output accordingly.
| let element: KeylessAuthElement | KeylessEnrollElement | null; | ||
|
|
||
| element = document.querySelector('kl-auth, kl-enroll'); | ||
| if (!element) throw new Error(); | ||
|
|
||
| recognizeElement = element; |
There was a problem hiding this comment.
Make the custom-element query type explicit.
Line 41 relies on querySelector returning the custom element union. Use const and an explicit generic/cast so the assignment is stable under TypeScript DOM typings.
🛠️ Proposed fix
- let element: KeylessAuthElement | KeylessEnrollElement | null;
-
- element = document.querySelector('kl-auth, kl-enroll');
+ const element = document.querySelector<KeylessAuthElement | KeylessEnrollElement>(
+ 'kl-auth, kl-enroll',
+ );
if (!element) throw new Error();🧰 Tools
🪛 ESLint
[error] 41-41: 'element' is never reassigned. Use 'const' instead.
(prefer-const)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/recognize/src/lib/recognize.ts` around lines 39 - 44, Replace the
loose-typed `element` lookup with a const and an explicit DOM generic/cast so
TypeScript knows the query returns our custom element union: call
`document.querySelector` with the generic type `KeylessAuthElement |
KeylessEnrollElement` (or cast the result) into a `const el` and then assign it
to `recognizeElement` after the null check; update the thrown Error to remain
unchanged but ensure you reference the previously named symbols
`element`/`recognizeElement`/`KeylessAuthElement`/`KeylessEnrollElement` so the
assignment is stable under TypeScript DOM typings.
| /** | ||
| * @property {string} [webSocketUrl] - the websocket url for the PingOne Recognize SDK to connect to; | ||
| */ | ||
| webSocketUrl: string; |
There was a problem hiding this comment.
Align the JSDoc with the required type.
webSocketUrl is required by the interface and by the SDK element configuration, but [webSocketUrl] documents it as optional.
📝 Proposed fix
- * `@property` {string} [webSocketUrl] - the websocket url for the PingOne Recognize SDK to connect to;
+ * `@property` {string} webSocketUrl - the websocket url for the PingOne Recognize SDK to connect to;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @property {string} [webSocketUrl] - the websocket url for the PingOne Recognize SDK to connect to; | |
| */ | |
| webSocketUrl: string; | |
| /** | |
| * `@property` {string} webSocketUrl - the websocket url for the PingOne Recognize SDK to connect to; | |
| */ | |
| webSocketUrl: string; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/recognize/src/lib/recognize.types.ts` around lines 34 - 37, The
JSDoc incorrectly marks webSocketUrl as optional while the interface property
webSocketUrl is required; update the comment for the webSocketUrl property in
recognize.types.ts to remove the optional brackets and reflect it's required
(e.g., change "@property {string} [webSocketUrl]" to "@property {string}
webSocketUrl") and ensure the description remains accurate for the PingOne
Recognize SDK connection URL.
| import { defineConfig } from 'vitest/config'; | ||
|
|
||
| export default defineConfig(() => ({ | ||
| root: __dirname, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Description: Verify ESM configs that still rely on CommonJS __dirname.
# Expected: No .mts/.mjs config files should use bare __dirname unless the repo intentionally bundles/polyfills it.
rg -n -C2 '\b__dirname\b' \
--iglob '*.mts' \
--iglob '*.mjs' \
--iglob 'vite.config.*' \
--iglob 'vitest.config.*' \
.
printf '\n-- Native ESM behavior check --\n'
node --input-type=module -e 'try { console.log(__dirname) } catch (error) { console.log(`${error.name}: ${error.message}`) }'Repository: ForgeRock/ping-javascript-sdk
Length of output: 8431
🏁 Script executed:
# Check if there's a package.json or build configuration that might affect module resolution
head -50 packages/recognize/package.json
# Look at the actual vitest.config.mts file to see its full content and any imports
cat packages/recognize/vitest.config.mts
# Check if there are any type: "module" or similar configurations that might affect this
fd -t f 'tsconfig' packages/recognize/ | xargs cat 2>/dev/null | head -30Repository: ForgeRock/ping-javascript-sdk
Length of output: 2385
🏁 Script executed:
# Check if vitest is run in CI and look for any errors
fd -t f "vitest" . --type f | head -20
# Look for test invocation in package.json or nx config
grep -r "vitest" packages/recognize/package.json
# Check root package.json for vitest scripts
grep -A5 -B5 "test" package.json | head -40
# Check if there are any actual issues reported in the repo about this
rg "ReferenceError.*__dirname" . --type log
# Search for any issue discussions or TODOs about __dirname in config
rg "__dirname" packages/recognize/ -A2 -B2Repository: ForgeRock/ping-javascript-sdk
Length of output: 1848
🏁 Script executed:
# Check the vitest.workspace.ts to see how configs are loaded
cat vitest.workspace.ts
# Look for any Vite/Vitest specific plugins or loaders that might provide __dirname
rg "global|__dirname|polyfill" vitest.config.mts packages/recognize/
# Check if tests actually run successfully - look for test output or CI config
fd -t f ".yml" . | xargs grep -l "vitest\|test" | head -5Repository: ForgeRock/ping-javascript-sdk
Length of output: 1237
🏁 Script executed:
# Critical finding - check if the .mts file is even being loaded
# The workspace pattern is '*.vitest.config.ts' but the file is .mts
# Check what actually exists in packages/recognize/
ls -la packages/recognize/ | grep -i config
# Check if there's a vitest.config.ts alongside the .mts
find packages/recognize -maxdepth 1 -name "*vitest.config*"
# Try to actually run vitest for this package to see if there are errors
cd packages/recognize && cat package.json | grep -A5 '"test"' 2>/dev/null || echo "No test script found"Repository: ForgeRock/ping-javascript-sdk
Length of output: 467
🏁 Script executed:
# The workspace pattern shows *.vitest.config.ts but the file is .mts
# This means the config may not be loaded at all!
# Check the workspace pattern more carefully
cat vitest.workspace.ts | grep -A5 -B5 "vite.config"
# See if there are any other .mts config files in the repo
find . -name "*vitest.config.mts" -o -name "*vite.config.mts"
# Check if this is by design - maybe the .mts file shouldn't exist at all
git log --oneline packages/recognize/vitest.config.mts 2>/dev/null | head -5 || echo "Git history not available"
# Check if there's documentation about .mts config files
rg ".mts" . --type md | head -20Repository: ForgeRock/ping-javascript-sdk
Length of output: 409
🏁 Script executed:
# Critical: The workspace pattern is for .ts files, not .mts
# Check if this config is even being discovered by vitest
# List all vitest configs in the workspace
find packages -name "*vitest.config*" -type f
# Check if other packages use .ts or .mts
grep -l "vitest.config" $(find packages -name "package.json" | head -10) 2>/dev/null
# Most importantly: does the pattern need to match .mts?
# Let's see if changing .mts to .ts would be the right fix
ls -la packages/*/vitest.config.* tools/*/vitest.config.* 2>/dev/null | grep -v "^d"Repository: ForgeRock/ping-javascript-sdk
Length of output: 459
🏁 Script executed:
# Verify: the .mts file is not being picked up by the workspace pattern
# Let's check if renaming to .ts or updating the pattern would be needed
# Check if there's a reason for .mts specifically
git log --all --oneline -- packages/recognize/vitest.config.mts | head -10
# See what commit introduced it
git show de09e0c:packages/recognize/vitest.config.mts 2>/dev/null | head -20
# Check if package.json type: "module" requires .mts
cat packages/recognize/package.json | grep -A2 -B2 '"type"'Repository: ForgeRock/ping-javascript-sdk
Length of output: 720
🏁 Script executed:
# Final verification: Check if Vite config loader supports .mts pattern matching
# and whether the config file is actually being loaded/discovered
# Look for any documentation or comments about .mts in vite/vitest config
rg "\.mts|mts" . --type ts --type json | grep -i "config\|pattern" | head -20
# Check the actual Vite/Vitest versions to understand config file support
grep -E "vitest|vite" package.json | head -10Repository: ForgeRock/ping-javascript-sdk
Length of output: 1732
Update vitest.config.mts to use ESM-safe root path and match workspace discovery pattern.
This file uses __dirname which is unavailable in ES modules. Additionally, the vitest.workspace.ts discovery pattern only matches .ts files ('**/*/vitest.config.ts'), not .mts, so this config won't be discovered by vitest. Fix both issues by renaming to .ts and deriving the root from import.meta.url:
♻️ Proposed fix
+import { fileURLToPath } from 'node:url';
import { defineConfig } from 'vitest/config';
+const root = fileURLToPath(new URL('.', import.meta.url));
+
export default defineConfig(() => ({
- root: __dirname,
+ root,Rename vitest.config.mts → vitest.config.ts
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/recognize/vitest.config.mts` around lines 1 - 4, The config
currently uses the CJS-only __dirname and a .mts extension that won't be
discovered by the workspace pattern; rename vitest.config.mts → vitest.config.ts
and update the root value to be ESM-safe by deriving it from import.meta.url
(e.g., convert import.meta.url to a file path and use its directory) so the
config runs under ESM and matches the existing workspace discovery pattern
'**/*/vitest.config.ts'.
ryanbas21
force-pushed
the
recognize-setup
branch
from
de09e0c to
54ab56e
Compare
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.
Nx Cloud has identified a possible root cause for your failed CI:
We identified that the failing test (@forgerock/oidc-suites:e2e-ci--src/user.spec.ts) is caused by a pre-existing environment issue — PingAM is returning an "invalid request_uri" OIDC error unrelated to the changes in this PR. Our investigation confirmed the same failure exists in branch 576, indicating this is not introduced by the current changes.
No code changes were suggested for this issue.
Trigger a rerun:
🎓 Learn more about Self-Healing CI on nx.dev
ryanbas21
force-pushed
the
recognize-setup
branch
from
54ab56e to
39ee8bf
Compare
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
|
Deployed 9eac0d0 to https://ForgeRock.github.io/ping-javascript-sdk/pr-580/9eac0d05f402850fe8d6b6444112356fab000c93 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/device-client - 0.0 KB (-10.0 KB, -100.0%) 🆕 New Packages🆕 @forgerock/recognize - 3853.5 KB (new) ➖ No Changes➖ @forgerock/storage - 1.5 KB 15 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (18.21%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #580 +/- ##
==========================================
+ Coverage 18.07% 18.21% +0.13%
==========================================
Files 155 155
Lines 24398 24440 +42
Branches 1203 1212 +9
==========================================
+ Hits 4410 4451 +41
- Misses 19988 19989 +1 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/recognize/src/lib/recognize.ts (2)
97-104: 💤 Low valueDocument the
key→publicKeymapping.Line 101 silently renames the config property
keytopublicKeywhen applying it to the element. Adding a brief comment explaining this special case would improve maintainability.📝 Suggested comment
const applyConfig = (el: RootEl): void => { const target = el as unknown as Record<string, unknown>; + // Map config.key → element.publicKey to match SDK element API for (const [k, v] of Object.entries(effectiveConfig)) { if (v !== undefined) { target[k === 'key' ? 'publicKey' : k] = v; } } };🤖 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/recognize/src/lib/recognize.ts` around lines 97 - 104, The applyConfig function silently renames the configuration property "key" to "publicKey" when copying entries from effectiveConfig into the element; add a concise inline comment above the conditional/assignment in applyConfig explaining that "key" is intentionally mapped to the DOM/RootEl property "publicKey" (e.g., to avoid reserved word conflicts or to match the element API), so future maintainers understand why target[k === 'key' ? 'publicKey' : k] is used; reference the applyConfig function and the key -> publicKey mapping in the comment.
129-129: 💤 Low valueConsider deduplicating the username assignment.
Both the
attachandmountbranches setelement.username = options.username;immediately after obtaining the element reference. You could extract this along withapplyConfigandattachListenersto reduce duplication.♻️ Proposed refactor
} else { const tag = options.type === 'auth' ? 'kl-auth' : 'kl-enroll'; element = document.createElement(tag) as RootEl; - element.username = options.username; - applyConfig(element); - attachListeners(element); options.container.appendChild(element); } + element.username = options.username; + applyConfig(element); + attachListeners(element); },Note: Move the common operations after both branches.
Also applies to: 135-135
🤖 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/recognize/src/lib/recognize.ts` at line 129, Both branches in the create/initialization flow duplicate common operations: setting element.username and calling applyConfig and attachListeners; refactor by moving element.username = options.username (and the subsequent applyConfig(...) and attachListeners(...)) out of both the attach and mount branches so they run once after you obtain the element reference. Locate the code around the attach and mount logic in recognize.ts (functions/methods referencing attach, mount, element, applyConfig, attachListeners) and replace the duplicated statements with a single post-branch block that assigns element.username and invokes applyConfig and attachListeners.e2e/recognize-app/vite.config.ts (1)
8-8: ⚡ Quick winPrefer consistent path construction.
Lines 8 and 21 use string concatenation for paths, while line 26 mixes
resolvewith string concatenation. For maintainability, useresolveorpath.joinconsistently.♻️ Proposed fix
- root: __dirname + '/src', + root: resolve(__dirname, 'src'), cacheDir: '../../node_modules/.vite/e2e/recognize-app', - publicDir: __dirname + '/public', + publicDir: resolve(__dirname, 'public'),build: { - outDir: __dirname + '/dist', + outDir: resolve(__dirname, 'dist'), emptyOutDir: true, reportCompressedSize: true, rollupOptions: { input: { - main: resolve(__dirname + '/src', 'index.html'), + main: resolve(__dirname, 'src', 'index.html'), },Also applies to: 21-21, 26-26
🤖 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 `@e2e/recognize-app/vite.config.ts` at line 8, Replace ad-hoc string concatenation for filesystem paths with a consistent path resolver (e.g., path.resolve or path.join): locate the root definition currently written as "root: __dirname + '/src'" and any other path concatenations in this file, import and use path.resolve(__dirname, 'src') (or path.join) for all path constructions (including the other two occurrences referenced) so all paths use the same method for consistency and maintainability.
🤖 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.
Nitpick comments:
In `@e2e/recognize-app/vite.config.ts`:
- Line 8: Replace ad-hoc string concatenation for filesystem paths with a
consistent path resolver (e.g., path.resolve or path.join): locate the root
definition currently written as "root: __dirname + '/src'" and any other path
concatenations in this file, import and use path.resolve(__dirname, 'src') (or
path.join) for all path constructions (including the other two occurrences
referenced) so all paths use the same method for consistency and
maintainability.
In `@packages/recognize/src/lib/recognize.ts`:
- Around line 97-104: The applyConfig function silently renames the
configuration property "key" to "publicKey" when copying entries from
effectiveConfig into the element; add a concise inline comment above the
conditional/assignment in applyConfig explaining that "key" is intentionally
mapped to the DOM/RootEl property "publicKey" (e.g., to avoid reserved word
conflicts or to match the element API), so future maintainers understand why
target[k === 'key' ? 'publicKey' : k] is used; reference the applyConfig
function and the key -> publicKey mapping in the comment.
- Line 129: Both branches in the create/initialization flow duplicate common
operations: setting element.username and calling applyConfig and
attachListeners; refactor by moving element.username = options.username (and the
subsequent applyConfig(...) and attachListeners(...)) out of both the attach and
mount branches so they run once after you obtain the element reference. Locate
the code around the attach and mount logic in recognize.ts (functions/methods
referencing attach, mount, element, applyConfig, attachListeners) and replace
the duplicated statements with a single post-branch block that assigns
element.username and invokes applyConfig and attachListeners.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fdd1e040-d8f5-4795-8da0-6dbb122971d0
⛔ Files ignored due to path filters (3)
packages/recognize/src/lib/recognize-sdk/pthreads/wasm.wasmis excluded by!**/*.wasmpackages/recognize/src/lib/recognize-sdk/wasm.wasmis excluded by!**/*.wasmpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
e2e/recognize-app/eslint.config.mjse2e/recognize-app/package.jsone2e/recognize-app/src/index.htmle2e/recognize-app/src/index.tse2e/recognize-app/src/styles.csse2e/recognize-app/tsconfig.app.jsone2e/recognize-app/tsconfig.jsone2e/recognize-app/vite.config.tspackages/recognize/README.mdpackages/recognize/eslint.config.mjspackages/recognize/package.jsonpackages/recognize/src/index.tspackages/recognize/src/lib/errors.tspackages/recognize/src/lib/recognize-sdk/index.d.tspackages/recognize/src/lib/recognize-sdk/index.jspackages/recognize/src/lib/recognize-sdk/pthreads/wasm.jspackages/recognize/src/lib/recognize-sdk/wasm.datapackages/recognize/src/lib/recognize-sdk/wasm.jspackages/recognize/src/lib/recognize.spec.tspackages/recognize/src/lib/recognize.tspackages/recognize/src/lib/recognize.types.tspackages/recognize/tsconfig.jsonpackages/recognize/tsconfig.lib.jsonpackages/recognize/tsconfig.spec.jsonpackages/recognize/vitest.config.mtstsconfig.json
✅ Files skipped from review due to trivial changes (7)
- packages/recognize/tsconfig.spec.json
- packages/recognize/README.md
- e2e/recognize-app/package.json
- tsconfig.json
- e2e/recognize-app/src/styles.css
- e2e/recognize-app/tsconfig.json
- e2e/recognize-app/src/index.html
🚧 Files skipped from review as they are similar to previous changes (9)
- e2e/recognize-app/tsconfig.app.json
- packages/recognize/src/lib/recognize.spec.ts
- packages/recognize/eslint.config.mjs
- packages/recognize/tsconfig.lib.json
- e2e/recognize-app/src/index.ts
- packages/recognize/tsconfig.json
- packages/recognize/vitest.config.mts
- packages/recognize/package.json
- e2e/recognize-app/eslint.config.mjs
3 participants
JIRA Ticket
N/A
Description
setup recognize app and package
Summary by CodeRabbit
New Features
E2E App
Tests
Documentation