diff --git a/.github/workflows/npm-cd-beta.yml b/.github/workflows/npm-cd-beta.yml index 46a6a82..340d1c9 100644 --- a/.github/workflows/npm-cd-beta.yml +++ b/.github/workflows/npm-cd-beta.yml @@ -3,6 +3,10 @@ on: push: tags: - "[0-9]+.[0-9]+.[0-9]+-beta.[0-9]+" + +permissions: + id-token: write + contents: read jobs: build: runs-on: ubuntu-latest @@ -12,15 +16,16 @@ jobs: run: corepack enable - name: Set Yarn Version run: corepack prepare yarn@4.7.0 --activate - - name: Use Node.js ${{ matrix.node-version }} + - name: Use Node.js 24.x uses: actions/setup-node@v4 with: - node-version: ${{ matrix.node-version }} + node-version: '24.x' + registry-url: 'https://registry.npmjs.org' cache: "yarn" cache-dependency-path: yarn.lock - name: Install modules run: yarn install + - run: yarn test - run: yarn build - - run: npm publish --tag beta - env: - NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + - name: Publish to npm with OIDC + run: NODE_AUTH_TOKEN="" npm publish --tag beta --provenance --access public diff --git a/package.json b/package.json index a003e92..75a906d 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@flagship.io/react-sdk", "sideEffects": false, - "version": "5.2.3", + "version": "5.2.4", "license": "Apache-2.0", "description": "Flagship REACT SDK", "main": "dist/src/index.js", diff --git a/src/hooks.ts b/src/hooks.ts index 4b82ba1..1207535 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1,13 +1,11 @@ 'use client'; -import { useRef, useEffect } from "react"; +import { useRef } from "react"; import { Visitor, primitive } from "./deps"; export function useLatestRef(value: T) { const ref = useRef(value); - useEffect(() => { - ref.current = value; - }, [value]); + ref.current = value; return ref; } diff --git a/src/sdkVersion.ts b/src/sdkVersion.ts index 65a919e..9850cb1 100644 --- a/src/sdkVersion.ts +++ b/src/sdkVersion.ts @@ -1,2 +1,2 @@ // Generated by genversion. -export const version = '5.2.3' +export const version = '5.2.4' diff --git a/test/StaleRefRepro.test.tsx b/test/StaleRefRepro.test.tsx new file mode 100644 index 0000000..51f748b --- /dev/null +++ b/test/StaleRefRepro.test.tsx @@ -0,0 +1,63 @@ +import React from 'react' + +import { jest, expect, it, describe, beforeEach, afterEach } from '@jest/globals' +import { renderHook } from '@testing-library/react-hooks' +import { Mock } from 'jest-mock' + +import { FSFlagCollection } from '@flagship.io/js-sdk' + +import * as FsHooks from '../src/FlagshipHooks' + +describe('repro: stale ref when reading getFlags() synchronously during render', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let realUseContext: (context: React.Context) => any + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let useContextMock: Mock<(context: React.Context) => any> + + beforeEach(() => { + realUseContext = React.useContext + useContextMock = jest.fn() + React.useContext = useContextMock + }) + + afterEach(() => { + React.useContext = realUseContext + }) + + it('getFlags() called inline in render should reflect the visitor from the SAME render, not a previous one', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const readSnapshots: any[] = [] + + // Render 1: no visitor yet, still fetching + useContextMock.mockReturnValue({ + state: { flagsStatus: { status: 'FETCHING', reason: 'NONE' }, flags: new Map() } + }) + + const { rerender } = renderHook(() => { + const hook = FsHooks.useFlagship() + // Mimic a consumer component that reads flags directly in its render body, + // e.g.: const { getFlags, flagsStatus } = useFlagship(); const flags = getFlags() + readSnapshots.push({ status: hook.flagsStatus, flags: hook.getFlags() }) + return hook + }) + + // Render 2: visitor becomes ready AND flagsStatus flips to FETCHED in the SAME state update + const visitor = { + getFlags: jest.fn(() => new FSFlagCollection({ flags: new Map([['my-flag', { key: 'my-flag' }]]) as any })) + } + useContextMock.mockReturnValue({ + state: { visitor, flagsStatus: { status: 'FETCHED', reason: 'NONE' } } + }) + + rerender() + + const lastSnapshot = readSnapshots[readSnapshots.length - 1] + + expect(lastSnapshot.status).toEqual({ status: 'FETCHED', reason: 'NONE' }) + // This is the actual bug: with the old useEffect-based useLatestRef, the ref + // is still pointing at the *previous* (visitor-less) render when getFlags() + // is read synchronously during render, so this comes back empty even though + // flagsStatus already says FETCHED. + expect(lastSnapshot.flags.size).toBe(1) + }) +})