Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[issue-#6] Include flag as is (no case formatting) #7

Merged

Conversation

tboulis
Copy link
Contributor

@tboulis tboulis commented Jun 9, 2021

Added optional second parameter to mockFlags() function that is of type:

type mockFlagsOptions = {
  skipFormatting?: boolean
}

If skipFormatting is true, the flags will be mocked without formatting them to camelCase and kebab-case.

Related to #6

EDIT #1:

Based on code review comment, the original flag is added as-is so the pristine value is returned

const kebab = kebabCase(k)
const camel = camelCase(k)
result[kebab] = flags[k]
result[camel] = flags[k]
Copy link
Collaborator

@yusinto yusinto Jan 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR and apologies for the long wait. Instead of skipping formatting (or aliasing) we can add the original flag key as-is so the pristine value is returned in this function. This way we can support all cases without having to set any extra options:

export const mockFlags = (flags: LDFlagSet) => {
  mockUseFlags.mockImplementation(() => {
    const result: LDFlagSet = {}
    Object.keys(flags).forEach((k) => {
      const kebab = kebabCase(k)
      const camel = camelCase(k)
      result[kebab] = flags[k]
      result[camel] = flags[k]
      result[k] = flags[k] // Add the original flag key and value here
    })
    return result
  })
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ok @yusinto , I've made the changes you suggested

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @tboulis ! I'll go ahead and merge this now. I will make a release soon. Thanks again.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more comment before I can merge this @tboulis please. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure @yusinto !

@tboulis tboulis changed the title Include options in mockFlags() in order to ignore case formatting. [issue-#6] Include flag as is (no case formatting) Jan 13, 2022
@tboulis tboulis requested a review from yusinto January 25, 2022 10:27
README.md Outdated Show resolved Hide resolved
Co-authored-by: Yusinto Ngadiman <[email protected]>
@yusinto yusinto merged commit dcaa2b0 into launchdarkly:master Feb 3, 2022
@yusinto
Copy link
Collaborator

yusinto commented Feb 3, 2022

This has been released. Thanks @tboulis

@tboulis tboulis deleted the enhancement/add-snake-case-support branch February 3, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants