Skip to content

Commit

Permalink
fix: generateGuestId edge case with hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
christianmat committed Dec 18, 2024
1 parent c79d0f5 commit 0a6c80d
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 18 deletions.
6 changes: 6 additions & 0 deletions .changeset/weak-trees-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@frigade/js": patch
"@frigade/react": patch
---

Fixes an issue where `generateGuestId` if used with the `useFrigade()` could cause components not to load
2 changes: 1 addition & 1 deletion apps/smithy/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const preview = {
apiKey={FRIGADE_API_KEY}
apiUrl={FRIGADE_API_URL}
defaultCollection={true}
// generateGuestId={false}
// generateGuestId={true}
userId={FRIGADE_USER_ID}
// syncOnWindowUpdates={false}

Expand Down
41 changes: 37 additions & 4 deletions packages/js-api/src/core/frigade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ export class Frigade extends Fetchable {
* @ignore
*/
private hasFailed = false
/**
* @ignore
*/
private lastSessionDTO?: SessionDTO

/**
* @ignore
Expand Down Expand Up @@ -112,7 +116,7 @@ export class Frigade extends Fetchable {
...config,
})

if (!this.config.userId && this.config.generateGuestId === false) {
if (!this.config.userId) {
return
}

Expand Down Expand Up @@ -146,13 +150,19 @@ export class Frigade extends Fetchable {
* @param properties
*/
public async identify(userId: string, properties?: PropertyPayload): Promise<void> {
await this.updateConfig({ ...this.config, userId })
if (userId !== this.config.userId) {
await this.updateConfig({ ...this.config, userId })
await this.reload()
}

await this.initIfNeeded()
await this.session({
const isNewSession = await this.session({
userId: this.config.userId,
userProperties: properties,
})
await this.resync()
if (isNewSession) {
await this.resync()
}
}

/**
Expand Down Expand Up @@ -217,10 +227,20 @@ export class Frigade extends Fetchable {
* @ignore
*/
private async session(sessionDTO: SessionDTO) {
const lastSessionDTO = this.lastSessionDTO

if (lastSessionDTO && JSON.stringify(lastSessionDTO) === JSON.stringify(sessionDTO)) {
return false
}

await this.fetch('/v1/public/sessions', {
method: 'POST',
body: JSON.stringify(sessionDTO),
})

this.lastSessionDTO = sessionDTO

return true
}

/**
Expand All @@ -235,6 +255,13 @@ export class Frigade extends Fetchable {
* @param flowId
*/
public async getFlow(flowId: string) {
if (
this.getConfig().generateGuestId === false &&
this.getConfig().userId &&
this.getConfig().userId.startsWith(GUEST_PREFIX)
) {
return undefined
}
await this.initIfNeeded()

return this.getFlowSync(flowId)
Expand All @@ -249,6 +276,12 @@ export class Frigade extends Fetchable {

public async getFlows() {
await this.initIfNeeded()
if (
this.config.generateGuestId === false &&
this.getConfig().userId?.startsWith(GUEST_PREFIX)
) {
return []
}
return this.flows
}

Expand Down
3 changes: 0 additions & 3 deletions packages/js-api/src/shared/fetchable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ export class Fetchable {

constructor(config: FrigadeConfig) {
const filteredConfig = Object.fromEntries(Object.entries(config).filter(([_, v]) => v != null))
if (!config.userId && config.generateGuestId === false) {
delete this.config.userId
}
this.config = {
...this.config,
...filteredConfig,
Expand Down
2 changes: 1 addition & 1 deletion packages/js-api/src/shared/state.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FlowStates, FlowStep, FrigadeConfig, StatefulFlow } from '../core/types'
import { FlowStates, FlowStep, FrigadeConfig, SessionDTO, StatefulFlow } from '../core/types'
import { Flow } from '../core/flow'
import type { Collections } from '../core/collections'

Expand Down
10 changes: 5 additions & 5 deletions packages/js-api/src/shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class CallQueue {
}
}

const callQueue = new CallQueue()
globalThis.callQueue = new CallQueue()

export async function gracefulFetch(
url: string,
Expand All @@ -137,7 +137,7 @@ export async function gracefulFetch(
const isWebPostRequest = isWeb() && options && options.body && options.method === 'POST'

if (isWebPostRequest && !cancelPendingRequests) {
const cachedCall = callQueue.hasIdenticalRecentCall(lastCallDataKey)
const cachedCall = globalThis.callQueue.hasIdenticalRecentCall(lastCallDataKey)

if (cachedCall != null && cachedCall.response != null) {
const cachedResponse = await cachedCall.response
Expand All @@ -149,21 +149,21 @@ export async function gracefulFetch(
if (!response) {
try {
if (cancelPendingRequests) {
callQueue.cancelAllPendingRequests()
globalThis.callQueue.cancelAllPendingRequests()
}

const pendingResponse = fetch(url, options)

if (isWebPostRequest) {
callQueue.push(
globalThis.callQueue.push(
lastCallDataKey,
// @ts-ignore
pendingResponse.then((res) => res.clone()).catch(() => getEmptyResponse())
)
}

response = await pendingResponse
if (isWebPostRequest && !callQueue.hasCall(lastCallDataKey)) {
if (isWebPostRequest && !globalThis.callQueue.hasCall(lastCallDataKey)) {
response = getEmptyResponse(REDUNDANT_CALL_MESSAGE)
}
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion packages/js-api/test/frigade.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('Basic Checklist integration test', () => {
})
let flows = await frigade.getFlows()
expect(flows.length).toEqual(0)
expect(frigade.isReady()).toBeFalsy()
expect(frigade.isReady()).toBeTruthy()
await frigade.identify(getRandomID())
flows = await frigade.getFlows()
expect(flows.length).toBeGreaterThan(0)
Expand Down
9 changes: 8 additions & 1 deletion packages/react/src/hooks/useGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export function useGroup() {
logOnce('useGroup() must be used in a child of the Frigade Provider', 'warn')
}
const { frigade } = context ?? {}
const groupId = frigade?.getConfig()?.groupId

/**
* Sets properties for the current group
Expand Down Expand Up @@ -40,5 +41,11 @@ export function useGroup() {
await frigade.group(groupId, properties)
}

return { groupId: frigade?.config?.groupId, setGroupId, addProperties, track }
return {
groupId,
setGroupId,
addProperties,
track,
isLoading: !frigade?.isReady(),
}
}
5 changes: 3 additions & 2 deletions packages/react/src/hooks/useUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export function useUser() {
if (!context || !context.frigade) {
logOnce('useUser() must be used in a child of the Frigade Provider', 'warn')
}
const { userId, frigade } = context ?? {}
const { frigade } = context ?? {}
const userId = frigade?.config.userId

/**
* Adds properties for the current user
Expand All @@ -27,5 +28,5 @@ export function useUser() {
await frigade.track(eventName, properties)
}

return { userId, addProperties, track }
return { userId, addProperties, track, isLoading: !frigade?.isReady() }
}

0 comments on commit 0a6c80d

Please sign in to comment.