-
Notifications
You must be signed in to change notification settings - Fork 25
fix: write conflicts enum export #288
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
Conversation
WalkthroughThe PR adds re-exports for Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (2)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
=======================================
Coverage 89.45% 89.46%
=======================================
Files 24 24
Lines 1299 1300 +1
Branches 234 213 -21
=======================================
+ Hits 1162 1163 +1
Misses 81 81
Partials 56 56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR fixes issue #286 by adding explicit named exports for the ClientWriteRequestOnDuplicateWrites and ClientWriteRequestOnMissingDeletes enum values to the root module. While these enums were previously exported as types through the wildcard export statement, TypeScript's export * only re-exports type declarations, not const value declarations, preventing users from accessing the runtime enum objects needed to use the conflict options documented in the README.
Key Changes:
- Added explicit named exports in index.ts for both ClientWriteRequestOnDuplicateWrites and ClientWriteRequestOnMissingDeletes enum values
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| export * from "./api"; | ||
| export * from "./client"; |
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.
Should this not export them?
Edit: never mind didn't read the PR body properly
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.
Exactly my thoughts initially but I tried it doesn't work
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.
Hmm are we sure about this, I did the following with the latest SDK and it works just fine:
import { ClientWriteRequestOnDuplicateWrites, ClientWriteRequestOnMissingDeletes } from '@openfga/sdk';
console.log(ClientWriteRequestOnDuplicateWrites);
console.log(ClientWriteRequestOnMissingDeletes);
Outputs
node index.mjs
{ Error: 'error', Ignore: 'ignore' }
{ Error: 'error', Ignore: 'ignore' }
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.
Yep it does seem to work now, I am not sure how I was able to reproduce it
Let me traceback and see I'll close this for now
Fixes #286
Description
This PR fixes issue #286 where the ClientWriteRequestOnDuplicateWrites and ClientWriteRequestOnMissingDeletes enums were not accessible to client code despite being documented in the README.
What problem is being solved?
Users were unable to import and use the ClientWriteRequestOnDuplicateWrites and ClientWriteRequestOnMissingDeletes enums in their code, even though the README documentation shows these enums should be importable and usable with the client.write() method's conflict options. When users attempted to import these enums from @openfga/sdk, they would receive undefined at runtime because the const value declarations were not being re-exported by the root module.
How is it being solved?
What changes are made to solve it?
The issue stems from TypeScript's export * statement behavior. In client.ts, both enums are declared as both const values and types:
TypeScript's export * from "./client" statement only re-exports type declarations, not const value declarations. This means while the type was available for TypeScript type checking, the actual runtime enum objects with their .Error and .Ignore properties were not accessible to users.
What changes are made to solve it?
Added explicit named exports in index.ts:
export { ClientWriteRequestOnDuplicateWrites, ClientWriteRequestOnMissingDeletes } from "./client";References
Review Checklist
mainSummary by CodeRabbit