Repository navigation
fix: keep default config for options passed as undefined - #97
Open
hey-research-lab wants to merge 1 commit into
Open
hey-research-lab wants to merge 1 commit into
hey-research-lab wants to merge 1 commit into
Conversation
new Codex(key, { apiUrl: process.env.CODEX_API_URL }) with the variable
unset replaced the default apiUrl with undefined, so every query and
mutation failed with "Only absolute URLs are supported". The same held
for apiRealtimeUrl and ws.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Codex.parseConfigmerges the caller's config over the defaults withObject.assign({}, defaultConfig, config).Object.assigncopies keys whose value isundefined, so an option that is present but undefined replaces its default. The examples inexamples/simple/construct the SDK asand the examples README only asks for
CODEX_API_KEY. WithCODEX_API_URLunset,apiUrlbecomesundefinedand every query and mutation fails:The same happens to
apiRealtimeUrl, andws: undefinedturns the websocket off.Fix
parseConfigdrops options whose value isundefinedbefore merging, so they keep their defaults. Explicit values (includingws: false) behave as before, andupdateConfiggoes through the same function.Validation
src/sdk/index.test.tsthat passesapiUrl,apiRealtimeUrlandwsasundefinedwith a stubfetch: onmainthe request goes to"undefined"(test fails); with the fix it goes tohttps://graph.codex.io/graphqland the websocket client gets the default URL withws: true.pnpm test(build:types + jest): 4 suites, 93 tests pass.eslintandprettier --checkon the changed files: no errors.