fix(sdk-rtl): widen IAccessToken.refresh_token to accept null - #1736
Open
marc-wilson wants to merge 1 commit into
Open
fix(sdk-rtl): widen IAccessToken.refresh_token to accept null#1736marc-wilson wants to merge 1 commit into
marc-wilson wants to merge 1 commit into
Conversation
`@looker/sdk`'s `IAccessToken.refresh_token` is `string | null | undefined`, matching the actual Looker API response shape, while `@looker/sdk-rtl`'s re-declared `IAccessToken` only allowed `string | undefined`. This meant a `login_user()` response couldn't be passed directly into `AuthToken.setToken()`/constructor without a manual `null -> undefined` cast. Widen sdk-rtl's `IAccessToken.refresh_token` to `string | null` to match. `AuthToken.setToken()` already treats a nil `refresh_token` (undefined or null) as "keep the existing refresh token", so this is a type-only change with no behavior difference. Fixes looker-open-source#1729
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.
What
@looker/sdkand@looker/sdk-rtlboth declare anIAccessTokeninterface with the same name but incompatible shapes forrefresh_token:@looker/sdk(packages/sdk/src/4.0/models.ts):refresh_token?: string | null— matches the actual Looker API response shape@looker/sdk-rtl(packages/sdk-rtl/src/authSession.ts):refresh_token?: string— narrower, re-declared independently of model generationThis meant a
login_user()response (typed with the sdk'sIAccessToken) couldn't be passed directly intoAuthToken.setToken()/constructor (typed with the rtl'sIAccessToken) without a manualnull -> undefinedcast at the call site:Fix
Widen
sdk-rtl'sIAccessToken.refresh_tokentostring | null, matching@looker/sdk's shape.@looker/sdkdepends on@looker/sdk-rtl(not the reverse), so widening the rtl-side declaration is the only direction that doesn't introduce a circular package dependency.This is a type-only change with no behavior difference:
AuthToken.setToken()already doesif (token.refresh_token) { ... }, sonullandundefinedare already treated identically at runtime — a nilrefresh_tokenmeans "keep using the existing refresh token."Verification
packages/sdk/src/4.0/accessTokenCompat.spec.ts, which constructs an@looker/sdkIAccessTokenwithrefresh_token: nulland passes it directly intosdk-rtl'sAuthTokenwith no cast.npx tsc -p tsconfig.json(full monorepo, source-level typecheck) — reproduced the exactTS2345error from this issue. Restored the fix and reran — zero errors across the whole repo.tsc -p tsconfig.jsonpasses cleanly with the fix in place.yarn buildpasses across all 21 packages.sdk,sdk-rtl,sdk-node,hackathon: same pass/fail counts on this branch as onmain(548 passed; 5 pre-existing failures require a livelooker.ini/Looker connection, unrelated to this change).Fixes #1729
Test plan
npx tsc -p tsconfig.json(full monorepo typecheck) passes with zero errorsyarn buildpasses cleanly across all 21 packagespackages/sdk/src/4.0/accessTokenCompat.spec.tspassessdk/sdk-rtl/sdk-node/hackathontest suites