launchdarkly / jest-launchdarkly-mock

Easily unit test LaunchDarkly feature flagged components with jest
Apache License 2.0
25 stars 19 forks source link

Support for testing React components that use variation() function #23

Closed pdrummond closed 2 years ago

pdrummond commented 2 years ago

Is your feature request related to a problem? Please describe.

We can't use the useFlags() hook as it incorrectly reports that flags are evaluated when they are fetched via allFlags(), even though they are not evaluated at that time. This is very confusing for our users of the Launch Darkly dashboard as the dashboard incorrectly states that old flags that are no longer even in our codebase were evaluated "2 mins ago" (for example).

The advice from your support team is to initialise with sendEventsOnlyForVariation set to true and always use ldClient.variation() to evaluate flags in components instead of useFlags().

However, it looks like this library doesn't support testing components that use ldClient.variation() yet.

Describe the solution you'd like

Add support so that it's possible to test components that use ldClient.variation() instead of useFlags().

yusinto commented 2 years ago

Hi @pdrummond thanks for submitting this. There is support right now to test ldClient.variation. If you use useLDClient() in your code base, then in your test you access the ldClientMock object and assert the variation function. For example:

import React from 'react'
import { render, fireEvent } from '@testing-library/react'
import { ldClientMock, resetLDMocks } from 'jest-launchdarkly-mock'

  it('should invoke variation correctly', () => {
    // Other setup code here....
    const { getByTestId } = render(<Button />)
    fireEvent.click(getByTestId('test-button'))

    expect(ldClientMock.variation).toBeCalledWith('dev-test-flag', false)
  })

That should work. Please try and let us know. Thank you.

pdrummond commented 2 years ago

Hi @yusinto. Thanks for your response but that's not going to solve my problem. I don't want to test that the variation() function has been called. I want to test the functionality of the component based on the flag being enabled using the variation() function.

In other words, how do I test this component using your library?

const Button: FunctionComponent = ({ children }) => {
  const ldClient = useLDClient() as LDClient

  return ldClient.variation('dev-test-flag') ? (
    <button data-testid="test-button">{children}</button>
  ) : (
    <>button disabled</>
  )
}
yusinto commented 2 years ago

The functions of ldClientMock object are jest mocks as well. You can use the standard mock* functions to mock variation:

import React from 'react'
import { render, fireEvent, screen } from '@testing-library/react'
import { ldClientMock, resetLDMocks } from 'jest-launchdarkly-mock'

// ... other boilerplate test suite code 

  it('should render button', () => {
    // mock the variation function to return true
    ldClientMock.variation.mockReturnValue(true)

    render(<Button />)

    expect(ldClientMock.variation).toBeCalledWith('dev-test-flag')
    expect(screen.getByTestId('test-button')).toBeDefined()
  })
pdrummond commented 2 years ago

Ah okay, thanks for your help @yusinto. It's not what I was hoping for, but I think that will be enough to get our tests working for now, thanks.

I think the main reason I find it confusing is that I'm being told the variation() function is the idiomatic way to evaluate flags with Launch Darkly, yet all the examples (including the one in this library) focus heavily on useFlags(). The use of variation() is curiously missing from all docs and examples, as if it's just a minor convenience function that won't be of interest to most users.

Is the example in this library going to be changed to use variation() at some point or am I still missing something? I can't think of a reason why anyone would prefer useFlags() over the variation() function as useFlags() causes the "Evaluated XX mins ago" analytics on the dashboard to be incorrect (as well as making all information in the insights tab incorrect) because the time it shows is actually when all the flags were fetched during initialisation which is very misleadingly and confusing for our users of the dashboard.

I know what I'm referring to isn't specific to this library but any advice you have on this would be extremely helpful as I still feel like I'm using LD incorrectly. Even just adding another example to this library (and the React SDK one) that uses variation() in the Button component would go a long way to providing some reassurance this is the recommended approach for users who care about insights/analytics.

yusinto commented 2 years ago

Hi @pdrummond apologies for the slow response. The way the React SDK is initialized by default is to request all flags via the ldClient.allFlags() function which will cause all flags to be "Evaluated xxx mins ago". If you only want to listen to a specific set of flags, not all flags, you can initialize the React SDK with the flags config option. You can also read about it more on our docs page.

Here's an example of how you can initialize the React SDK with only the flags which are of interest:

  const LDProvider = await asyncWithLDProvider({
    clientSideID: 'your-client-side-id',
    flags: {
      'my-bool-flag': true,
      'my-int-flag': 5,
    },
  });
pdrummond commented 2 years ago

Hi @yusinto. Thanks for your response but that won't work for us. If I am reading the LD React SDK code correctly, your example above does indeed call variation under the hood instead of allFlags but it's still evaluating the flags at initialisation time rather than when the flag is actually evaluated and we still get incorrect analytics because of that. Also, this approach means every time we add a flag we would need to keep a separate list of flags updated which isn't too much effort but still feels like an overhead and a completely unnecessary one at that.

It's okay though - we have solved the problem by implementing our own custom useFlags hook that simply invokes variation at the time of flag evaluation. I will add an issue to the LD React SDK repo to suggest this is should be the default for your useFlags hook as well and once you provide that we'll retire our custom hook, but until then it's working well for us.

Also - just some feedback for you if it's useful - we couldn't get this library working with create-react-app so we've implemented a custom mocking solution as well.

yusinto commented 2 years ago

Thanks @pdrummond. The React SDK team will be looking at that issue. In the meantime, are you able to open a separate issue for the create-react-app problem please? So we can investigate and correct it for the benefit of others. Thank you. I'll close this issue now.

HarrisonLeach1 commented 2 years ago

Hey @yusinto my team is in a similar situation where we prefer using .variation() in a custom hook over useFlags() because it provides us with better flag name safety through use of enums and better analytics.

This package would be more helpful to us if ldClientMock.variation() had mocked return values based on the flag set passed to mockFlags(). What do you think of this? Is this a contribution you would accept?