-
Notifications
You must be signed in to change notification settings - Fork 0
fix: exports #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: exports #34
Conversation
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
WalkthroughThe pull request updates REST model exports in the main index to use namespace exports for both Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Comment |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/assets/navigation.js(1 hunks)docs/assets/search.js(1 hunks)docs/interfaces/restCycleballModel.components.html(1 hunks)docs/interfaces/restModel.components.html(1 hunks)docs/interfaces/restModel.operations.html(1 hunks)docs/interfaces/restModel.paths.html(1 hunks)docs/modules/restCycleballModel.html(1 hunks)docs/modules/restModel.html(1 hunks)docs/types/_defs.html(0 hunks)docs/types/restCycleballModel._defs.html(1 hunks)docs/types/restCycleballModel.webhooks.html(1 hunks)docs/types/restModel._defs.html(1 hunks)docs/types/restModel.webhooks.html(1 hunks)src/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- docs/types/_defs.html
🔇 Additional comments (12)
docs/assets/search.js (1)
1-1: Data-only update, no logic touched. LGTM.docs/assets/navigation.js (1)
1-1: Data-only update, no logic touched. LGTM.docs/types/restCycleballModel._defs.html (1)
1-1: New TypeDoc page added; consistent with namespace docs.docs/types/restModel.webhooks.html (1)
1-1: New TypeDoc page added; consistent with restModel namespace.docs/interfaces/restCycleballModel.components.html (1)
1-7: Generated docs look consistent with new restCycleballModel namespace.
If possible, add a usage snippet in docs showing:
- Old: N/A
- New:
import type { restCycleballModel } from '@taimos/radball-digital-api';docs/interfaces/restModel.paths.html (1)
1-6: Doc note addition is fine; rest is generated.docs/modules/restModel.html (1)
1-1: New restModel namespace page matches export change.
Consider adding a short import example in the landing page to guide consumers post-change.docs/interfaces/restModel.operations.html (1)
1-1: Generated documentation files cannot be meaningfully reviewed.These are TypeDoc-generated HTML artifacts. Review should focus on the source code changes in
src/index.tsrather than the generated documentation output. The namespace pathrestModelin the breadcrumb suggests the export reorganization was applied, but verification of the source code is needed.docs/interfaces/restModel.components.html (1)
1-1: Generated documentation—skip detailed review.Like the previous file, this is auto-generated TypeDoc output. Verification of the underlying source changes in
src/index.tsis the priority.docs/types/restModel._defs.html (1)
1-1: Generated documentation—skip detailed review.docs/modules/restCycleballModel.html (1)
1-1: Generated documentation for new namespace.The new
restCycleballModelnamespace appears correctly in the documentation structure with expected Interfaces (components, operations, paths) and Type Aliases ($defs, webhooks). Verify thatsrc/index.tsexports this new namespace correctly.docs/types/restCycleballModel.webhooks.html (1)
1-1: Generated documentation—skip detailed review.
| export * as restModel from './generated/rest.model.generated'; | ||
| export * as restCycleballModel from './generated/rest.cycleball.model.generated'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Potential breaking change + possible runtime import of type‑only modules.
- Moving to
export * as restModel/restCycleballModelchanges import shape for consumers (e.g.,import type { paths } from '@taimos/radball-digital-api'→import type { restModel } from '@taimos/radball-digital-api'; type P = restModel.paths). Consider this a breaking change and either keep a compatibility re‑export or bump major and add migration notes. export * as …emits a runtime namespace import. If./generated/rest*.model.generatedare declaration‑only (no emitted .js), this will break at runtime/build.
Suggested minimal compatibility fix (keep new namespaces; restore legacy star export):
export * from './generated/graphql.model.generated';
+export * from './generated/rest.model.generated'; // temporary: preserve old import style
export * as restModel from './generated/rest.model.generated';
export * as restCycleballModel from './generated/rest.cycleball.model.generated';
export * from './validation';Also add a short “Migration” section to README and plan removal in the next major.
Run to verify build setup and artifacts:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check generated REST model artifacts =="
fd -a 'rest\.model\.generated\.(ts|js|d\.ts)' generated || true
fd -a 'rest\.cycleball\.model\.generated\.(ts|js|d\.ts)' generated || true
echo -e "\n== tsconfig flags that affect emits =="
for f in $(fd -a 'tsconfig*.json'); do
echo "---- $f ----"
jq '.compilerOptions | {module,target,declaration,emitDeclarationOnly,verbatimModuleSyntax,preserveValueImports}' "$f" 2>/dev/null || cat "$f"
done
echo -e "\n== Grep for old import style (may be in samples/docs) =="
rg -nP "from\s+['\"]@taimos/radball-digital-api['\"]" -C2 --type=ts --type=tsx || trueLength of output: 1391
Now checking actual codebase structure and usage:
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "== Check for REST model files anywhere in src/ =="
find src -name "*rest*model*" -type f 2>/dev/null || echo "No rest model files found"
echo -e "\n== Check full src/index.ts and surrounding files =="
find src -maxdepth 2 -type f -name "*.ts" | head -20
echo -e "\n== Search for any imports of @taimos/radball-digital-api in src/index.ts and tests =="
rg -n "from.*@taimos" --type=ts -C2 || echo "No imports found"
Length of output: 167
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "== List root directory =="
ls -la . | head -30
echo -e "\n== Find tsconfig or package.json =="
find . -maxdepth 2 -name "tsconfig.json" -o -name "package.json" | head -10
echo -e "\n== Look for index.ts anywhere =="
find . -name "index.ts" -type f 2>/dev/null | head -10
Length of output: 1927
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "== Full src/index.ts content =="
cat ./src/index.ts
echo -e "\n== List src/ directory =="
ls -la ./src/
echo -e "\n== List src/generated/ if exists =="
ls -la ./src/generated/ 2>/dev/null || echo "src/generated not found"
echo -e "\n== Search test/ and samples/ for imports of models =="
rg -n "import.*from.*\./(generated|index)" ./test ./samples 2>/dev/null || echo "No direct imports found"
echo -e "\n== Check package.json for entry points and exports field =="
jq '.main, .exports, .types' ./package.json
Length of output: 1202
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "== Check package.json name and publishing settings =="
jq '.name, .private, .publishConfig' ./package.json
echo -e "\n== Check README for migration/deprecation notes =="
head -80 ./README.md
echo -e "\n== Look for CHANGELOG or migration docs =="
ls -la ./ | grep -iE "CHANGELOG|MIGRATION|BREAKING|HISTORY"
echo -e "\n== Inspect one of the generated files to see export structure =="
head -30 ./src/generated/rest.model.generated.ts
Length of output: 2359
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "== Check what types/interfaces are exported from rest.model.generated =="
rg "^export (type|interface|const)" ./src/generated/rest.model.generated.ts | head -20
echo -e "\n== Check git log for any version bumps or breaking change notes =="
git log --oneline --all -20 -- src/index.ts 2>/dev/null | head -10 || echo "No git history"
echo -e "\n== Check if there are any test files that import the models =="
find ./test -name "*.ts" -type f -exec grep -l "from.*index\|from.*generated" {} \; 2>/dev/null || echo "No test imports found"
Length of output: 454
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "== Git diff for 'fix: exports' commit =="
git show 8b5a67b --stat 2>/dev/null || echo "Cannot get diff"
echo -e "\n== Check test/validation.test.ts imports =="
head -20 ./test/validation.test.ts
echo -e "\n== Check if git branch is PR branch or main =="
git branch -a 2>/dev/null | head -10 || echo "Cannot get branch info"
git log -1 --oneline 2>/dev/null || echo "Cannot get current commit"
Length of output: 2287
Breaking change in public API confirmed—remove incorrect declaration-only concern and address migration strategy.
The export change is a legitimate breaking change for the public npm package @taimos/radball-digital-api:
-
Breaking change (confirmed): Consumers currently importing
import { paths } from '@taimos/radball-digital-api'must switch toimport { restModel } from '@taimos/radball-digital-api'; type P = restModel.paths. The README's example import style will fail after this change. -
Declaration-only concern (incorrect):
tsconfig.jsonhasdeclaration: truewithmodule: "CommonJS"andoutDir: "lib", which emits both .js and .d.ts files. Theexport * as namespacepattern is valid and produces proper runtime re-exports in CommonJS—no runtime failure expected. -
Suggested fix issues: While adding a compatibility re-export (
export * from './generated/rest.model.generated') softens the impact, it creates API inconsistency (graphql flat vs. REST namespaced). Consider instead updating the README with new import patterns and documenting this as a breaking change requiring a major version bump.
Summary by CodeRabbit
Documentation
Breaking Changes