From 3861277417fb46bf025f0ad536486ec68bce9255 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Mon, 1 Mar 2021 14:50:15 -0600 Subject: [PATCH] feat(feature-flags): rename exports, add merge, and update tests (#7802) * feat(feature-flags): rename exports, add merge, and update tests * feat(feature-flags): add support for process.env checking for flags * feat(feature-flags): add sass module variant with behavior that matches JS * feat(project): integrate feature-flags with react package * docs(feature-flags): update README Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/feature-flags/README.md | 103 ++++++------ .../__tests__/feature-flags-test.js | 40 ----- packages/feature-flags/__tests__/scss-test.js | 157 ++++++++++++++---- packages/feature-flags/feature-flags.yml | 5 + packages/feature-flags/index.scss | 64 +++++++ packages/feature-flags/package.json | 7 +- .../feature-flags/scss/feature-flags.scss | 30 ---- .../src/__tests__/feature-flags-test.js | 104 ++++++++++++ packages/feature-flags/src/index.js | 22 ++- packages/feature-flags/tasks/build.js | 13 +- packages/react/package.json | 1 + packages/react/src/internal/FeatureFlags.js | 6 +- yarn.lock | 4 +- 13 files changed, 389 insertions(+), 167 deletions(-) delete mode 100644 packages/feature-flags/__tests__/feature-flags-test.js create mode 100644 packages/feature-flags/index.scss delete mode 100644 packages/feature-flags/scss/feature-flags.scss create mode 100644 packages/feature-flags/src/__tests__/feature-flags-test.js diff --git a/packages/feature-flags/README.md b/packages/feature-flags/README.md index 939018cb1585..966d2fd7a335 100644 --- a/packages/feature-flags/README.md +++ b/packages/feature-flags/README.md @@ -24,83 +24,76 @@ The `@carbon/feature-flags` provides a runtime-based feature flag system that you can use to enable or disable experimental features from Carbon or in your own code. -To enable a feature flag, you will need to enable it in each of the environments -that you're using it in. Often times, this will mean JavaScript, Sass, or both. - -To enable a flag in JavaScript, you would use `enableFeatureFlag`: +To check if a feature flag is enabled, you can use the `enabled` function in +JavaScript: ```js -import { enableFeatureFlag } from '@carbon/feature-flags'; +import { enabled } from '@carbon/feature-flags'; -enableFeatureFlag('feature-flag-name'); +enabled('feature-flag-name'); ``` -To enable a flag in Sass, you would set the `$feature-flags` variable: +In Sass, you would use the `enabled` function or mixin: ```scss -@import '@carbon/feature-flags/scss/feature-flags'; +@use '@carbon/feature-flags'; -$feature-flags: map-merge( - $feature-flags, - ( - 'feature-flag-name': true, - ) -); -``` +// Return true if the flag is enabled +@if feature-flags.enabled('feature-flag-name') { + // +} -### Managing and using feature flags +@include enabled('feature-flag-name') { + // only include contents if the flag is enabled +} +``` -You can use the `@carbon/feature-flags` package to build on top of existing -feature flags, or to add your own. +### Managing feature flags -You can add and toggle flags in JavaScript and Sass. In JavaScript, this would -look like: +You can change whether a feature flag is enabled. In JavaScript, you can use the +`enable`, `disable`, and `merge` functions to accomplish this. ```js -import { - addFeatureFlag, - enableFeatureFlag, - disableFeatureFlag, - featureFlagEnabled, -} from '@carbon/feature-flags'; - -// Specify a default value for the flag -addFeatureFlag('feature-flag-name', false); - -// You can use `featureFlagEnabled` to conditionally run -// branches of your code -if (featureFlagEnabled('feature-flag-name')) { - // Run code if the flag is enabled -} +import { enable, disable, merge } from '@carbon/feature-flags'; -// You can also modify the value of the flag -disableFeatureFlag('feature-flag-name'); -enableFeatureFlag('feature-flag-name'); +// Enable `feature-flag-a` +enable('feature-flag-a'); + +// Disable `feature-flag-a` +disable('feature-flag-a); + +// Set a variety of feature flags to a specific value +merge({ + 'feature-flag-a': true, + 'feature-flag-b': false, + 'feature-flag-c': true, +}); ``` -In Sass, you would write the following: +In Sass, you can configure whether a feature flag is enabled when you include +the module or by using `enable`, `disable`, and `merge`. ```scss -@import '@carbon/feature-flags/scss/feature-flags'; +@use '@carbon/feature-flags' with ( + $feature-flags: ( + 'feature-flag-a': false, + 'feature-flag-b': true, + ), +); + +// Enable `feature-flag-a` +@include feature-flags.enable('feature-flag-a'); + +// Disable `feature-flag-b` +@include feature-flags.disable('feature-flag-b'); -$feature-flags: map-merge( - $feature-flags, +// Set a variety of feature flags to a specific value +@include feature-flags.merge( ( - 'feature-flag-name': true, + 'feature-flag-a': true, + 'feature-flag-b': true, ) ); - -@if feature-flag-enabled('feature-flag-name') { - // ... -} - -// You can also run this as a mixin to conditionally include -// code -.my-selector { - @include feature-flag-enabled('feature-flag-name') { - // ... - } -} ``` ## 🙌 Contributing diff --git a/packages/feature-flags/__tests__/feature-flags-test.js b/packages/feature-flags/__tests__/feature-flags-test.js deleted file mode 100644 index 236ef6ecc7a9..000000000000 --- a/packages/feature-flags/__tests__/feature-flags-test.js +++ /dev/null @@ -1,40 +0,0 @@ -/** - * Copyright IBM Corp. 2015, 2020 - * - * This source code is licensed under the Apache-2.0 license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { - addFeatureFlag, - enableFeatureFlag, - disableFeatureFlag, - featureFlagEnabled, -} from '../src'; - -describe('@carbon/feature-flags', () => { - it('should let the user check if a feature flag is enabled', () => { - addFeatureFlag('test', false); - expect(featureFlagEnabled('test')).toBe(false); - - enableFeatureFlag('test'); - expect(featureFlagEnabled('test')).toBe(true); - - disableFeatureFlag('test'); - expect(featureFlagEnabled('test')).toBe(false); - }); - - it('should throw an error if a flag does not exist', () => { - expect(() => { - enableFeatureFlag('does-not-exist'); - }).toThrow(); - - expect(() => { - disableFeatureFlag('does-not-exist'); - }).toThrow(); - - expect(() => { - featureFlagEnabled('does-not-exist'); - }).toThrow(); - }); -}); diff --git a/packages/feature-flags/__tests__/scss-test.js b/packages/feature-flags/__tests__/scss-test.js index 8fc3d72d6dda..129e472f02b4 100644 --- a/packages/feature-flags/__tests__/scss-test.js +++ b/packages/feature-flags/__tests__/scss-test.js @@ -9,49 +9,142 @@ 'use strict'; -const { convert, createSassRenderer } = require('@carbon/test-utils/scss'); +const { SassRenderer } = require('@carbon/test-utils/scss'); -const render = createSassRenderer(__dirname); +const { render } = SassRenderer.create(__dirname); -describe('feature-flags.scss', () => { - it('should support default feature flags before the import', async () => { - const { calls } = await render(` - $feature-flags: ('test': true); +describe('@carbon/feature-flags', () => { + describe('add', () => { + it('should add the given flag and set whether its enabled', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; - @import '../scss/feature-flags'; + @include feature-flags.add(flag-a, true); + @include feature-flags.add(flag-b, false); - @if feature-flag-enabled('test') { - $t: test(true); - } - `); + $_: get-value(feature-flags.enabled(flag-a)); + $_: get-value(feature-flags.enabled(flag-b)); + `); - expect(calls.length).toBe(1); - expect(convert(calls[0][0])).toBe(true); + // flag-a + expect(getValue(0)).toBe(true); + // flag-b + expect(getValue(1)).toBe(false); + }); }); - it('should support modifying flags', async () => { - const { calls } = await render(` - @import '../scss/feature-flags'; + describe('enable', () => { + it('should enable the given feature flag', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; - $feature-flags: map-merge($feature-flags, ( - 'test': true, - )); + @include feature-flags.add(flag-a, false); - @if feature-flag-enabled('test') { - $t: test('feature-flag-enabled'); - } + $_: get-value(feature-flags.enabled(flag-a)); - $feature-flags: map-merge($feature-flags, ( - 'test': false, - )); + @include feature-flags.enable(flag-a); - @if not feature-flag-enabled('test') { - $t: test('feature-flag-disabled'); - } - `); + $_: get-value(feature-flags.enabled(flag-a)); + `); - expect(calls.length).toBe(2); - expect(convert(calls[0][0])).toBe('feature-flag-enabled'); - expect(convert(calls[1][0])).toBe('feature-flag-disabled'); + expect(getValue(0)).toBe(false); + expect(getValue(1)).toBe(true); + }); }); + + describe('disable', () => { + it('should disable the given feature flag', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; + + @include feature-flags.add(flag-a, true); + + $_: get-value(feature-flags.enabled(flag-a)); + + @include feature-flags.disable(flag-a); + + $_: get-value(feature-flags.enabled(flag-a)); + `); + + expect(getValue(0)).toBe(true); + expect(getValue(1)).toBe(false); + }); + }); + + describe('enabled', () => { + it('should return whether a flag is enabled or disabled', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; + + @include feature-flags.add(flag-a, true); + @include feature-flags.add(flag-b, false); + + $_: get-value(feature-flags.enabled(flag-a)); + $_: get-value(feature-flags.enabled(flag-b)); + `); + + // flag-a + expect(getValue(0)).toBe(true); + // flag-b + expect(getValue(1)).toBe(false); + }); + }); + + describe('merge', () => { + it('should set each feature flag given', async () => { + const { getValue } = await render(` + @use '../index.scss' as feature-flags; + + @include feature-flags.add(flag-c, false); + @include feature-flags.merge(( + flag-a: true, + flag-b: false, + flag-c: true, + )); + + $_: get-value(feature-flags.enabled(flag-a)); + $_: get-value(feature-flags.enabled(flag-b)); + $_: get-value(feature-flags.enabled(flag-c)); + `); + + // flag-a + expect(getValue(0)).toBe(true); + // flag-b + expect(getValue(1)).toBe(false); + // flag-c + expect(getValue(2)).toBe(true); + }); + }); + + // it('should support default feature flags before the import', async () => { + // const { calls } = await render(` + // $feature-flags: ('test': true); + // @import '../scss/feature-flags'; + // @if feature-flag-enabled('test') { + // $t: test(true); + // } + // `); + // expect(calls.length).toBe(1); + // expect(convert(calls[0][0])).toBe(true); + // }); + // it('should support modifying flags', async () => { + // const { calls } = await render(` + // @import '../scss/feature-flags'; + // $feature-flags: map-merge($feature-flags, ( + // 'test': true, + // )); + // @if feature-flag-enabled('test') { + // $t: test('feature-flag-enabled'); + // } + // $feature-flags: map-merge($feature-flags, ( + // 'test': false, + // )); + // @if not feature-flag-enabled('test') { + // $t: test('feature-flag-disabled'); + // } + // `); + // expect(calls.length).toBe(2); + // expect(convert(calls[0][0])).toBe('feature-flag-enabled'); + // expect(convert(calls[1][0])).toBe('feature-flag-disabled'); + // }); }); diff --git a/packages/feature-flags/feature-flags.yml b/packages/feature-flags/feature-flags.yml index 59a369e4acdc..333f2d4f3ab6 100644 --- a/packages/feature-flags/feature-flags.yml +++ b/packages/feature-flags/feature-flags.yml @@ -9,3 +9,8 @@ feature-flags: - name: enable-css-custom-properties description: Describe what the flag does enabled: false + - name: enable-use-controlled-state-with-value + description: > + Enable components to be created in either a controlled or uncontrolled + mode + enabled: false diff --git a/packages/feature-flags/index.scss b/packages/feature-flags/index.scss new file mode 100644 index 000000000000..2725779c0f28 --- /dev/null +++ b/packages/feature-flags/index.scss @@ -0,0 +1,64 @@ +// +// Copyright IBM Corp. 2015, 2020 +// +// This source code is licensed under the Apache-2.0 license found in the +// LICENSE file in the root directory of this source tree. +// + +@use 'sass:map'; +@use 'scss/generated/feature-flags'; + +$feature-flags: () !default; +$feature-flags: map.merge( + feature-flags.$generated-feature-flags, + $feature-flags +); + +@function check-for-flag($name) { + @if map.has-key($feature-flags, $name) == true { + @return true; + } + @error 'Unable to find a feature flag named: #{$name}'; +} + +@mixin add($name, $enabled: false) { + @if map.has-key($feature-flags, $name) == true { + @error 'A feature flag named #{$name} already exists'; + } + + $feature-flags: map.set($feature-flags, $name, $enabled) !global; +} + +@mixin enable($name) { + @if check-for-flag($name) == true { + $feature-flags: map.set($feature-flags, $name, true) !global; + } +} + +@mixin disable($name) { + @if check-for-flag($name) == true { + $feature-flags: map.set($feature-flags, $name, false) !global; + } +} + +@mixin merge($flags) { + $feature-flags: map.merge($feature-flags, $flags) !global; +} + +/// Check if a feature flag is enabled +/// @param {String} $name +/// @returns {Boolean} +@function enabled($name) { + @if check-for-flag($name) == true { + @return map.get($feature-flags, $name); + } +} + +/// Emit the content of the mixin if the feature flag is enabled +/// @param {String} $name +/// @content +@mixin enabled($name) { + @if enabled($name) { + @content; + } +} diff --git a/packages/feature-flags/package.json b/packages/feature-flags/package.json index 4d411f0f96cc..848a287905ee 100644 --- a/packages/feature-flags/package.json +++ b/packages/feature-flags/package.json @@ -1,8 +1,7 @@ { "name": "@carbon/feature-flags", - "private": true, "description": "Build with feature flags in Carbon", - "version": "0.5.0", + "version": "0.0.0", "license": "Apache-2.0", "main": "lib/index.js", "module": "es/index.js", @@ -28,10 +27,12 @@ "@babel/generator": "^7.10.2", "@babel/types": "^7.10.2", "@carbon/scss-generator": "^10.13.0", + "change-case": "^4.1.2", "fs-extra": "^9.0.1", "js-yaml": "^3.14.0", "rimraf": "^3.0.2", "rollup": "^2.38.0", "rollup-plugin-strip-banner": "^2.0.0" - } + }, + "sideEffects": false } diff --git a/packages/feature-flags/scss/feature-flags.scss b/packages/feature-flags/scss/feature-flags.scss deleted file mode 100644 index f73c95d9b50f..000000000000 --- a/packages/feature-flags/scss/feature-flags.scss +++ /dev/null @@ -1,30 +0,0 @@ -// -// Copyright IBM Corp. 2015, 2020 -// -// This source code is licensed under the Apache-2.0 license found in the -// LICENSE file in the root directory of this source tree. -// - -@import 'generated/feature-flags'; - -$feature-flags: () !default; -$feature-flags: map-merge($generated-feature-flags, $feature-flags); - -/// Check if a feature flag is enabled -/// @param {String} $name -/// @returns {Boolean} -@function feature-flag-enabled($name) { - @if (map-has-key($feature-flags, $name)) { - @return map-get($feature-flags, $name); - } - @error 'Unable to find feature flag named #{$name}'; -} - -/// Emit the content of the mixin if the feature flag is enabled -/// @param {String} $name -/// @content -@mixin feature-flag-enabled($name) { - @if feature-flag-enabled($name) { - @content; - } -} diff --git a/packages/feature-flags/src/__tests__/feature-flags-test.js b/packages/feature-flags/src/__tests__/feature-flags-test.js new file mode 100644 index 000000000000..45e3dfb89c74 --- /dev/null +++ b/packages/feature-flags/src/__tests__/feature-flags-test.js @@ -0,0 +1,104 @@ +/** + * Copyright IBM Corp. 2015, 2020 + * + * This source code is licensed under the Apache-2.0 license found in the + * LICENSE file in the root directory of this source tree. + */ + +describe('@carbon/feature-flags', () => { + let FeatureFlag; + + beforeEach(() => { + jest.resetModules(); + FeatureFlag = require('../'); + }); + + describe('add', () => { + it('should add the given flag and set whether its enabled', () => { + FeatureFlag.add('flag-a', true); + FeatureFlag.add('flag-b', false); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + + it('should throw if a duplicate flag name is given', () => { + FeatureFlag.add('flag-a', true); + + expect(() => { + FeatureFlag.add('flag-a', true); + }).toThrow(); + }); + }); + + describe('enable', () => { + it('should enable the given feature flag', () => { + FeatureFlag.add('flag-a', false); + expect(FeatureFlag.enabled('flag-a')).toBe(false); + + FeatureFlag.enable('flag-a'); + expect(FeatureFlag.enabled('flag-a')).toBe(true); + }); + + it('should throw if the given flag does not exist', () => { + expect(() => { + FeatureFlag.enable('flag-a'); + }).toThrow(); + }); + }); + + describe('disable', () => { + it('should disable the given feature flag', () => { + FeatureFlag.add('flag-a', true); + expect(FeatureFlag.enabled('flag-a')).toBe(true); + + FeatureFlag.disable('flag-a'); + expect(FeatureFlag.enabled('flag-a')).toBe(false); + }); + + it('should throw if the given flag does not exist', () => { + expect(() => { + FeatureFlag.disable('flag-a'); + }).toThrow(); + }); + }); + + describe('enabled', () => { + it('should return whether a flag is enabled or disabled', () => { + FeatureFlag.add('flag-a', true); + FeatureFlag.add('flag-b', false); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + + it('should throw if the given flag does not exist', () => { + expect(() => { + FeatureFlag.enabled('flag-a'); + }).toThrow(); + }); + }); + + describe('merge', () => { + it('should set each feature flag given', () => { + FeatureFlag.merge({ + 'flag-a': true, + 'flag-b': false, + }); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + + it('should override duplicate keys with the given flag', () => { + FeatureFlag.add('flag-b', true); + FeatureFlag.merge({ + 'flag-a': true, + 'flag-b': false, + }); + + expect(FeatureFlag.enabled('flag-a')).toBe(true); + expect(FeatureFlag.enabled('flag-b')).toBe(false); + }); + }); +}); diff --git a/packages/feature-flags/src/index.js b/packages/feature-flags/src/index.js index b4c2da9f630d..b6119e1c2b30 100644 --- a/packages/feature-flags/src/index.js +++ b/packages/feature-flags/src/index.js @@ -29,7 +29,10 @@ function checkForFlag(name) { * @param {string} name * @param {boolean} enabled */ -export function addFeatureFlag(name, enabled) { +export function add(name, enabled) { + if (featureFlags.has(name)) { + throw new Error(`The feature flag: ${name} already exists`); + } featureFlags.set(name, enabled); } @@ -37,7 +40,7 @@ export function addFeatureFlag(name, enabled) { * Enable a feature flag * @param {string} name */ -export function enableFeatureFlag(name) { +export function enable(name) { checkForFlag(name); featureFlags.set(name, true); } @@ -46,17 +49,28 @@ export function enableFeatureFlag(name) { * Disable a feature flag * @param {string} name */ -export function disableFeatureFlag(name) { +export function disable(name) { checkForFlag(name); featureFlags.set(name, false); } +/** + * Merge the given feature flags with the current set of feature flags. + * Duplicate keys will be set to the value in the given feature flags. + * @param {object} flags + */ +export function merge(flags) { + Object.keys(flags).forEach((key) => { + featureFlags.set(key, flags[key]); + }); +} + /** * Check if a feature flag is enabled * @param {string} name * @returns {boolean} */ -export function featureFlagEnabled(name) { +export function enabled(name) { checkForFlag(name); return featureFlags.get(name); } diff --git a/packages/feature-flags/tasks/build.js b/packages/feature-flags/tasks/build.js index 01919115b2b6..8589bc8a910b 100644 --- a/packages/feature-flags/tasks/build.js +++ b/packages/feature-flags/tasks/build.js @@ -10,6 +10,7 @@ const { default: babelGenerate } = require('@babel/generator'); const babelTypes = require('@babel/types'); const { types: t, generate } = require('@carbon/scss-generator'); +const { constantCase } = require('change-case'); const fs = require('fs-extra'); const yaml = require('js-yaml'); const path = require('path'); @@ -96,7 +97,17 @@ function buildJavaScriptModule(featureFlags) { ), t.objectProperty( t.identifier('enabled'), - t.booleanLiteral(featureFlag.enabled) + t.logicalExpression( + '||', + t.memberExpression( + t.memberExpression( + t.identifier('process'), + t.identifier('env') + ), + t.identifier(constantCase(`CARBON_${featureFlag.name}`)) + ), + t.booleanLiteral(featureFlag.enabled) + ) ), ]); }) diff --git a/packages/react/package.json b/packages/react/package.json index 3d22de3382dc..36fe5ae33f93 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -44,6 +44,7 @@ "react-dom": "^16.8.6 || ^17.0.1" }, "dependencies": { + "@carbon/feature-flags": "^0.0.0", "@carbon/icons-react": "^10.26.0", "@carbon/telemetry": "0.0.0-alpha.6", "classnames": "2.2.6", diff --git a/packages/react/src/internal/FeatureFlags.js b/packages/react/src/internal/FeatureFlags.js index de593328aee8..48fe0108ea57 100644 --- a/packages/react/src/internal/FeatureFlags.js +++ b/packages/react/src/internal/FeatureFlags.js @@ -5,6 +5,8 @@ * LICENSE file in the root directory of this source tree. */ +import { enabled } from '@carbon/feature-flags'; + /** * This file contains the list of the default values of compile-time feature flags. * @@ -44,4 +46,6 @@ * * `rest` tells you additional information based on the source component * * _Without_ this feature flag the event handler has component-specific signature, e.g. `onChange(event, direction)`. */ -export const useControlledStateWithValue = false; +export const useControlledStateWithValue = enabled( + 'enable-use-controlled-state-with-value' +); diff --git a/yarn.lock b/yarn.lock index 2762b7b9e7ff..7bcc67636fef 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1676,13 +1676,14 @@ __metadata: languageName: unknown linkType: soft -"@carbon/feature-flags@workspace:packages/feature-flags": +"@carbon/feature-flags@^0.0.0, @carbon/feature-flags@workspace:packages/feature-flags": version: 0.0.0-use.local resolution: "@carbon/feature-flags@workspace:packages/feature-flags" dependencies: "@babel/generator": ^7.10.2 "@babel/types": ^7.10.2 "@carbon/scss-generator": ^10.13.0 + change-case: ^4.1.2 fs-extra: ^9.0.1 js-yaml: ^3.14.0 rimraf: ^3.0.2 @@ -9229,6 +9230,7 @@ __metadata: "@babel/plugin-transform-object-assign": ^7.7.4 "@babel/preset-env": ^7.10.0 "@babel/preset-react": ^7.10.0 + "@carbon/feature-flags": ^0.0.0 "@carbon/icons-react": ^10.26.0 "@carbon/telemetry": 0.0.0-alpha.6 "@carbon/test-utils": ^10.15.0