From de5f637400a7ecfa7d8df4e5e0ccdde820add2da Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Fri, 1 May 2020 16:34:39 -0700 Subject: [PATCH 1/2] Test deprecations and remove in prod --- jest.config.js | 2 +- package.json | 1 + src/Flash.js | 17 +++++---- src/__tests__/Flash.js | 6 +++ src/utils/deprecate.js | 67 +++++++++++++++++++++++++++++++++- src/utils/test-deprecations.js | 18 +++++++++ yarn.lock | 5 +++ 7 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 src/utils/test-deprecations.js diff --git a/jest.config.js b/jest.config.js index 9a1b0e31b34..63e6363fac3 100644 --- a/jest.config.js +++ b/jest.config.js @@ -2,5 +2,5 @@ module.exports = { cacheDirectory: '.test', collectCoverage: true, collectCoverageFrom: ['src/*.js'], - setupFilesAfterEnv: ['/src/utils/test-matchers.js'] + setupFilesAfterEnv: ['/src/utils/test-matchers.js', '/src/utils/test-deprecations.js'] } diff --git a/package.json b/package.json index 4edf767eb9f..c9560e0854a 100644 --- a/package.json +++ b/package.json @@ -91,6 +91,7 @@ "rollup-plugin-babel": "4.4.0", "rollup-plugin-commonjs": "10.1.0", "rollup-plugin-terser": "5.3.0", + "semver": "7.3.2", "styled-components": "4.4.0" }, "peerDependencies": { diff --git a/src/Flash.js b/src/Flash.js index c557e9549ee..94fb1b941c2 100644 --- a/src/Flash.js +++ b/src/Flash.js @@ -5,7 +5,7 @@ import {variant} from 'styled-system' import {COMMON, get} from './constants' import theme from './theme' import sx from './sx' -import deprecate from './utils/deprecate' +import {useDeprecation} from './utils/deprecate' const schemeMap = { red: 'danger', @@ -44,14 +44,17 @@ const StyledFlash = styled.div` ` const Flash = ({variant, scheme, ...props}) => { + const deprecate = useDeprecation({ + name: 'Flash "scheme" prop', + version: '20.0.0', + message: 'Use the variant prop instead. See https://primer.style/components/Flash for more details.' + }) + if (scheme) { - deprecate({ - name: 'The scheme prop', - version: '20.0.0', - message: 'Use the variant prop instead. See https://primer.style/components/Flash for more details.' - }) + deprecate() variant = schemeMap[scheme] - } // deprecate 20.0.0 + } + return } diff --git a/src/__tests__/Flash.js b/src/__tests__/Flash.js index 73309fea1e9..32767c9a653 100644 --- a/src/__tests__/Flash.js +++ b/src/__tests__/Flash.js @@ -5,6 +5,7 @@ import theme, {colors} from '../theme' import {render, behavesAsComponent, checkExports} from '../utils/testing' import {render as HTMLRender, cleanup} from '@testing-library/react' import {axe, toHaveNoViolations} from 'jest-axe' +import {Deprecations} from '../utils/deprecate' import 'babel-polyfill' expect.extend(toHaveNoViolations) @@ -28,6 +29,11 @@ describe('Flash', () => { expect(render()).toHaveStyleRule('border-width', '1px 0px') }) + it('respects the deprecated "scheme" prop', () => { + expect(render()).toHaveStyleRule('background-color', colors.green[1]) + expect(Deprecations.getDeprecations()).toHaveLength(1) + }) + it('respects the "variant" prop', () => { expect(render()).toHaveStyleRule('background-color', colors.yellow[1]) expect(render()).toHaveStyleRule('background-color', '#FFE3E6') diff --git a/src/utils/deprecate.js b/src/utils/deprecate.js index 3a223cf406e..c1755aaf73f 100644 --- a/src/utils/deprecate.js +++ b/src/utils/deprecate.js @@ -1,5 +1,68 @@ /* eslint-disable no-console */ +import {useRef, useCallback} from 'react' -export default function deprecate({name, message, version}) { - console.warn(`WARNING! ${name} is deprecated and will be removed in version ${version}. ${message}`) +const noop = () => {} +// eslint-disable-next-line import/no-mutable-exports +let deprecate = null + +if (__DEV__) { + deprecate = ({name, message, version}) => { + Deprecations.deprecate({name, message, version}) + } +} else { + deprecate = noop +} + +export {deprecate} + +// eslint-disable-next-line import/no-mutable-exports +let useDeprecation = null + +if (__DEV__) { + useDeprecation = ({name, message, version}) => { + const ref = useRef(false) + const logDeprecation = useCallback(() => { + if (!ref.current) { + ref.current = true + deprecate({name, message, version}) + } + }, [name, message, version]) + + return logDeprecation + } +} else { + useDeprecation = () => { + return noop + } +} + +export {useDeprecation} + +export class Deprecations { + static get() { + if (!Deprecations.instance) { + Deprecations.instance = new Deprecations() + } + + return Deprecations.instance + } + + constructor() { + this.deprecations = [] + } + + static deprecate({name, message, version}) { + const msg = `WARNING! ${name} is deprecated and will be removed in version ${version}. ${message}` + console.warn(msg) + + this.get().deprecations.push({name, message, version}) + } + + static getDeprecations() { + return this.get().deprecations + } + + static clearDeprecations() { + this.get().deprecations.length = 0 + } } diff --git a/src/utils/test-deprecations.js b/src/utils/test-deprecations.js new file mode 100644 index 00000000000..6b5f85a3179 --- /dev/null +++ b/src/utils/test-deprecations.js @@ -0,0 +1,18 @@ +import semver from 'semver' +import {Deprecations} from '../utils/deprecate' + +const ourVersion = require('../../package.json').version + +beforeEach(() => { + Deprecations.clearDeprecations() +}) + +afterEach(() => { + const deprecations = Deprecations.getDeprecations() + + for (const dep of deprecations) { + if (semver.gte(ourVersion, dep.version)) { + throw new Error(`Found a deprecation that should be removed in ${dep.version}`) + } + } +}) diff --git a/yarn.lock b/yarn.lock index 57d023d64ca..00dd32676b0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10059,6 +10059,11 @@ semver@7.0.0: resolved "https://registry.yarnpkg.com/semver/-/semver-7.0.0.tgz#5f3ca35761e47e05b206c6daff2cf814f0316b8e" integrity sha512-+GB6zVA9LWh6zovYQLALHwv5rb2PHGlJi3lfiqIHxR0uuwCgefcOJc59v9fv1w8GbStwxuuqqAjI9NMAOOgq1A== +semver@7.3.2: + version "7.3.2" + resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.2.tgz#604962b052b81ed0786aae84389ffba70ffd3938" + integrity sha512-OrOb32TeeambH6UrhtShmF7CRDqhL6/5XpPNp2DuRH6+9QLw/orhp72j87v8Qa1ScDkvrrBNpZcDejAirJmfXQ== + semver@^6.0.0, semver@^6.1.2, semver@^6.2.0, semver@^6.3.0: version "6.3.0" resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d" From 9bef137cd60e340be3f5566ace294ea52cbc262f Mon Sep 17 00:00:00 2001 From: Michelle Tilley Date: Mon, 4 May 2020 15:25:59 -0700 Subject: [PATCH 2/2] Add notes on silencing deprecation warnings --- docs/content/getting-started.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/content/getting-started.md b/docs/content/getting-started.md index 1cc0fd91c5c..f3255d56a23 100644 --- a/docs/content/getting-started.md +++ b/docs/content/getting-started.md @@ -138,3 +138,7 @@ In your `tsconfig.json`, set the `types` array under the `compilerOptions` like ``` Of course, customize the array based on the `@types/` packages you have installed for your project. + +## Silencing warnings + +Like React, Primer Components emits warnings to the JavaScript console under certain conditions, like using deprecated components or props. Similar to React, you can silence these warnings by setting the `NODE_ENV` environment variable to `production` during bundling.