Repository navigation
Conversation
Ensure cookie methods (get, getAll, set, remove, setAll) are functions rather than merely checking property existence with the in operator, which evaluates to true for explicitly undefined properties. Guard the setAll server wrapper against undefined or omitted headers to prevent TypeError in Object.keys. Forward setAll calls to preserve the cookie adapter method receiver, fixing supabase#299. Add unit tests verifying behavior when accessors are explicitly undefined, when headers are omitted, and preserving receiver on stateful adapters.
This branch has not been deployed
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 kind of change does this PR introduce?
Bug fix.
What is the current behavior?
setAll,get,set,remove) are explicitly set toundefined(or non-function values) on thecookiesoptions object, the library previously only used theinoperator ("setAll" in cookies,"set" in cookies, etc.). Because"key" in { key: undefined }evaluates totruein JavaScript:createServerClient, omittingsetAlllogs a helpful warning on token refresh, but passing{ getAll: () => [], setAll: undefined }(common with conditional adapter configurations) bypassed this fallback and threwTypeError: originalSetAll is not a function.createBrowserClient, passing{ getAll: () => [], setAll: undefined }bypassed the initialization check and crashed later on storage write withTypeError: setAll is not a function.cookies: { get: () => ..., set: undefined, remove: undefined }attempted to callcookies.setand threwTypeError: cookies.set is not a function.this, callingsetAllas an unbound reference drops its method receiver, throwingTypeError: Cannot set properties of undefined(fixing cookies.setAll loses its receiver for stateful cookie adapters #299).createServerClient, the internalsetAllwrapper callsObject.keys(headers).length. Ifheadersis omitted or nullish, it crashes withTypeError: Cannot convert undefined or null to object.What is the new behavior?
get,getAll,set,remove,setAll) are actual functions before attempting invocation, preservinginfor TypeScript union type narrowing while guaranteeing runtime safety when properties are explicitlyundefined.cookies.setAll.setAllserver wrapper withBoolean(headers && Object.keys(headers).length > 0)to handle omitted or optional headers cleanly.setAllisundefined, and ensurescreateBrowserClientenforcessetAllpresence at construction time.Verification
pnpm exec vitest run: 125/125 tests pass (5 new unit tests added covering all cases).pnpm run build: bothtscbuilds (ESM & CJS) compile cleanly with 0 errors.pnpm exec eslint: 0 lint errors or warnings.pnpm exec prettier --check src/cookies.ts src/cookies.spec.ts src/createBrowserClient.spec.ts: cleanly formatted.