-
Notifications
You must be signed in to change notification settings - Fork 5
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/api url #55
Fix/api url #55
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EnvironmentSupplier
participant TokenSupplier
Client->>FlatfileClient: Initialize with options
FlatfileClient->>FlatfileClient: Check options.environment
alt if options.environment is undefined
FlatfileClient->>EnvironmentSupplier: Get environment
end
FlatfileClient->>FlatfileClient: Check options.token
alt if options.token is undefined
FlatfileClient->>TokenSupplier: Get token
end
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
package.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "@flatfile/api", | |||
"version": "1.9.18", | |||
"version": "1.9.19-rc.0", |
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.
"version": "1.9.19-rc.0", | |
"version": "1.9.18", |
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: 0
🧹 Outside diff range and nitpick comments (1)
src/wrapper/FlatfileClient.ts (1)
43-43
: Simplified environment resolution logic.The change streamlines the
resolveEnvironment
function by directly returningoptions.environment || options.apiUrl
. This simplification is good and aligns with the deprecation of theenvironment
option.However, to improve clarity and maintain consistency with the deprecation notice, consider using the nullish coalescing operator (
??
) instead of the logical OR (||
):return options.environment ?? options.apiUrl;This change would prioritize
options.apiUrl
whenoptions.environment
is null or undefined, which seems to be the intended behavior based on the deprecation notice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package.json
is excluded by!**/*.json
📒 Files selected for processing (1)
- src/wrapper/FlatfileClient.ts (2 hunks)
🔇 Additional comments (2)
src/wrapper/FlatfileClient.ts (2)
27-27
: Improved environment resolution with fallback.This change enhances the robustness of the
FlatfileClient
constructor by adding a fallback toenvironmentSupplier
whenresolveEnvironment(options)
returns undefined. This ensures that there's always a valid environment, improving the reliability of the client initialization process.
27-27
: Please provide more context on the API URL issue.The changes improve the environment resolution process, which seems to be related to the "Fix/api url" objective mentioned in the PR title. However, it would be helpful to have more information about the specific API URL issue these changes are addressing.
Could you please provide:
- A brief description of the API URL issue that prompted these changes?
- How do these modifications specifically resolve the problem?
- Are there any additional tests or verifications we should perform to ensure the fix is complete?
This additional context will help ensure that the changes fully address the intended problem and that no further modifications are necessary.
Also applies to: 43-43
This fixes an issue where the
apiUrl
/environment
value was stored permanently in theFlatfileClient
instead of re-fetching the value from the env valuesCloses https://github.com/FlatFilers/support-triage/issues/1575