Merge pull request #971 from primer/variable-deprecations

Add variable deprecation data and tests
2024-12-14 23:12:03 +03:00 · 2019-11-04 15:12:38 -08:00 · 2019-11-04 15:12:38 -08:00 · 4fec93975f
commit 4fec93975f
parent c539fcd9bb c6be855358
6 changed files with 251 additions and 33 deletions
--- a/deprecations.js
+++ b/deprecations.js
@ -4,6 +4,36 @@
 * array and a "message" string.
 */
 const versionDeprecations = {
+  '14.0.0': [
+    {
+      selectors: ['.UnderlineNav-item.selected', '.UnderlineNav-item.selected .UnderlineNav-octicon'],
+      message: `Please use aria-selected="true" to indicate the selected state of an UnderlineNav item.`
+    },
+    {
+      variables: ['$status-pending'],
+      message: `This variable is deprecated.`
+    },
+    {
+      variables: ['$repo-private-text', '$repo-private-bg', '$repo-private-icon'],
+      message: `These variables are deprecated.`
+    },
+    {
+      variables: ['$marketingSpacers', '$allSpacers'],
+      message: `Please use the $marketing-spacers and $marketing-all-spacers variables.`
+    },
+    {
+      variables: ['$exploregrid-item-border-radius'],
+      message: `This variable is deprecated. Use "4px" instead.`
+    },
+    {
+      variables: ['$stats-switcher-py', '$stats-viewport-height'],
+      message: `These variables are deprecated.`
+    },
+    {
+      variables: ['$min_tab_size', '$max_tab_size'],
+      message: `These variables are deprecated.`
+    }
+  ],
  '13.0.0': [
    {
      selectors: [
@ -68,11 +98,15 @@ const semver = require('semver')

 // map selectors to the version and message of their deprecation
 const selectorDeprecations = new Map()
+const variableDeprecations = new Map()
 for (const [version, deps] of Object.entries(versionDeprecations)) {
-  for (const {selectors, message} of deps) {
+  for (const {selectors = [], variables = [], message} of deps) {
    for (const selector of selectors) {
      selectorDeprecations.set(selector, {version, message})
    }
+    for (const variable of variables) {
+      variableDeprecations.set(variable, {version, message})
+    }
  }
 }

@ -81,4 +115,15 @@ function isSelectorDeprecated(selector, version = CURRENT_VERSION) {
  return deprecation ? semver.gte(deprecation.version, version) : false
 }

-module.exports = {versionDeprecations, selectorDeprecations, isSelectorDeprecated}
+function isVariableDeprecated(variable, version = CURRENT_VERSION) {
+  const deprecation = variableDeprecations.get(variable)
+  return deprecation ? semver.gte(deprecation.version, version) : false
+}
+
+module.exports = {
+  versionDeprecations,
+  selectorDeprecations,
+  variableDeprecations,
+  isSelectorDeprecated,
+  isVariableDeprecated
+}
--- a/docs/content/getting-started/contributing.md
+++ b/docs/content/getting-started/contributing.md
@ -42,7 +42,11 @@ It's usually better to open an issue before investing time in spiking out a new
 1. What the pattern is and how it's being used across the site - post screenshots and urls where possible. If you need help identifying where the pattern is being used, call that out here and cc the relevant team and/or cc `@product-design` to help.
 2. Why you think a new pattern is needed (this should answer the relevant questions above).
 3. If you intend to work on these new styles yourself, let us know what your timeline and next steps are. If you need help and this is a dependency for shipping another project, please make that clear here and what the timeline is.
-4. Add the `type: new styles` label, or `type: refactor` where appropriate.
+4. Add the appropriate label(s):
+    - `Type: Enhancement` for new styles
+    - `Type: Bug Fix` for—you guessed it!—bug fixes
+    - `Type: Polish` for refactors of existing styles
+    - `Type: Breaking Change` for any change that [removes CSS selectors or SCSS variables](#removing-styles-and-variables)

 ### Step 2: Design and build the new styles

@ -64,6 +68,67 @@ If you get to this step you've helped contribute to a style guide that many of y

 Let the [design systems team](https://github.com/github/design-systems) know if we can improve these guidelines and make following this process any easier.

+
+## Removing styles and variables
+
+Removing styles and SCSS variables can be scary. How do you know if the thing you're deleting (or just planning to delete) isn't used in other projects? [Semantic versioning] provides us with an answer: We **don't** know, but we can use "major" version increments (from, say, `13.4.0` to `14.0.0`) to signal that the release includes potentially breaking changes. The rule is simple:
+
+**Whenever we delete a CSS selector or SCSS variable, we will increment to the next major version.**
+
+When planning to delete a CSS selector or SCSS variable, you should:
+
+1. Add a [TODO@version comment](#primer-csstodo) above the line in question:
+
+    ```scss
+    // TODO@15.0.0: delete $some-unused-var
+    $some-unused-var: 15px !default;
+    ```
+
+1. Add it to [deprecations.js]:
+
+    ```js
+    const versionDeprecations = {
+      '15.0.0': [
+        {
+          variables: ['$some-unused-var'],
+          message: '$some-unused-var is unused, and has been deprecated.'
+        }
+      ]
+    }
+    ```
+
+We have several checks and tools in place to help us plan, track, and catch both expected and unexpected removals of both CSS selectors and SCSS variables:
+
+### `deprecations.js`
+[This file][deprecations.js] is where we document all of our current and _planned_ CSS selector and SCSS variable deprecations (removals), and is used to generate [deprecation data](../tools/deprecations) for other tools.
+
+### `script/test-deprecations.js`
+[This script][script/test-deprecations.js] compares the CSS stats and variable data between the latest release and the local code, and throws error messages if:
+
+- A CSS selector has been deleted but was not listed in [deprecations.js]
+- A CSS selector listed in [deprecations.js] was _not removed_ in the version it claims to have been deprecated
+- An SCSS variable has been deleted but was not listed in [deprecations.js]
+- An SCSS variable listed in [deprecations.js] was _not removed_ in the version it claims to have been deprecated
+
+Run `script/test-deprecation.js --help` for more info and available options.
+
+### `primer-css/TODO`
+[This stylelint rule][lib/stylelint-todo.js] looks for comments in the form:
+
+```scss
+// TODO@<version>: <message>
+```
+
+and generates an error for each one whose `<version>` is less than or equal to the current version (in `package.json`). You can test this rule for future releases with:
+
+```sh
+PRIMER_VERSION=<version> npx stylelint-only primer-css/TODO -- src
+```
+
+where `<version>` is the future version you'd like to compare against. Assuming that the correctly formatted comments exist already, violations of this stylelint rule can be used to generate a checklist of lines to remove in a future release.
+
+See [the deprecation data docs](../tools/deprecations) for more information.
+
 ## Documentation structure

 - Our documentation site for Primer CSS is built using [Doctocat](https://primer.style/doctocat) and deployed with [Now](https://zeit.co/now). Our site is built from the `docs` folder and uses [MDX](https://mdxjs.com) to render markdown.
@ -97,3 +162,8 @@ Check out Doctocat's [live code](https://primer.style/doctocat/usage/live-code)
 Primer CSS follows [semantic versioning](http://semver.org/) conventions. This helps others know when a change is a patch, minor, or breaking change.

 To understand what choice to make, you'll need to understand semver and know if one of the changes shown is a major, minor, or patch. Semver is confusing at first, so I recommend reviewing [semver](http://semver.org/) and/or ask in [#design-systems](https://github.slack.com/archives/design-systems) or and experienced open-source contributor.
+
+[semantic versioning]: https://semver.org
+[script/test-deprecations.js]: https://github.com/primer/css/tree/master/script/test-deprecations.js
+[deprecations.js]: https://github.com/primer/css/tree/master/deprecations.js
+[lib/stylelint-todo.js]: https://github.com/primer/css/tree/master/lib/stylelint-todo.js
--- a/docs/content/tools/deprecations.md
+++ b/docs/content/tools/deprecations.md
@ -2,28 +2,25 @@
 title: Deprecation data
 ---

-As of version 12.7.0, we publish CSS selector deprecation data with
-`@primer/css`. You can access the data via the [Node API](#node) or as
-[JSON](#json).
+As of version 12.7.0, we publish CSS selector and SCSS variable deprecation data (as of 14.0.0) with `@primer/css`. You can access the data via the [Node API](#node) or as [JSON](#json).

-Deprecation messages strings may include Markdown so that they can be included
-in the [changelog].
-
-**Keep in mind that this data includes both active and _planned_
-deprecations.** You can determine whether a CSS selector is deprecated for the
-version of `@primer/css` you've installed via the [Node API](#node), or by
-comparing the version of a selector deprecation with the installed version in
-your own environment.
+**Keep in mind that this data includes both active and _planned_ deprecations.** The [Node API](#node) is the best way to determine whether a selector or variable is deprecated for the version of `@primer/css` you've installed.

 ## JSON

 The JSON data is available in the unpacked node module's `dist/deprecations.json`, and is an object with the following structure:

-* `versions` is an object whose keys are version numbers (e.g. `13.0.0`) and values are deprecation messages: objects with a `selectors` array and a `message`:
+* `versions` is an object whose keys are version numbers (e.g. `13.0.0`) and values are deprecation messages, each of which has a `message` string and a `selectors` and/or `variables` array:

    ```json
    {
      "versions": {
+        "14.0.0": [
+          {
+            "variables": ["$min_tab_size", "$max_tab_size"],
+            "message": "These variables have been deprecated."
+          }
+        ],
        "13.0.0": [
          {
            "selectors": [".btn-purple"],
@ -33,6 +30,8 @@ The JSON data is available in the unpacked node module's `dist/deprecations.json
      }
    }
    ```
+    
+    Deprecation messages strings may include Markdown so that they can be included in the [changelog].

 * `selectors` is an object mapping CSS selectors (e.g. `.btn-purple`) to the version in which they are _or will be_ deprecated:

@ -46,6 +45,20 @@ The JSON data is available in the unpacked node module's `dist/deprecations.json
      }
    }
    ```
+    
+* `variables` is an object mapping SCSS variables (including the leading `$`, e.g. `$status-pending`) to the version in which they are or will be deprecated:
+
+    ```json
+    {
+      "variables": {
+        "$status-pending": {
+          "version": "14.0.0",
+          "message": "This variable is unused in Primer, and is deprecated."
+        }
+      }
+    }
+    ```
+    

 ## Node

@ -90,6 +103,27 @@ console.log(`Primary buttons are bad? ${isSelectorDeprecated('.btn-primary')}`)
 // "Primary buttons are bad? false"
 ```

+### `variableDeprecations`
+This is a [Map] object with keys for each SCSS variable mapped to the deprecation info:
+
+```js
+const {selectorDeprecations} = require('@primer/css/deprecations')
+console.log(`Will $status-pending be deprecated? ${variableDeprecations.has('$status-pending')}`)
+// "Will $status-pending be deprecated? true"
+```
+
+### `isVariableDeprecated(variable[, version])`
+Returns `true` if the named SCSS variable (including the leading `$`) will have been deprecated (removed) _by_ the specified [semver] version.
+
+```js
+const {isVariableDeprecated} = require('@primer/css/deprecations')
+console.log(`$status-pending deprecated? ${isVariableDeprecated('$status-pending')}`)
+// "$status-pending deprecated? true"
+console.log(`$yellow-700 deprecated? ${isVariableDeprecated('$yellow-700')}`)
+// "$yellow-700 deprecated false"
+```
+
+
 [semver]: https://npm.im/semver
 [changelog]: https://github.com/primer/css/tree/master/CHANGELOG.md
 [Map]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
--- a/script/dist.js
+++ b/script/dist.js
@ -88,15 +88,20 @@ function getPathName(path) {
 }

 function writeDeprecationData() {
-  const {versionDeprecations, selectorDeprecations} = require('../deprecations')
+  const {versionDeprecations, selectorDeprecations, variableDeprecations} = require('../deprecations')
  const data = {
    versions: versionDeprecations,
-    selectors: Array.from(selectorDeprecations.entries()).reduce((obj, [selector, deprecation]) => {
-      obj[selector] = deprecation
+    selectors: mapToObject(selectorDeprecations),
+    variables: mapToObject(variableDeprecations)
+  }
+  return writeFile(join(outDir, 'deprecations.json'), JSON.stringify(data, null, 2))
+
+  function mapToObject(map) {
+    return Array.from(map.entries()).reduce((obj, [key, value]) => {
+      obj[key] = value
      return obj
    }, {})
  }
-  return writeFile(join(outDir, 'deprecations.json'), JSON.stringify(data, null, 2))
 }

 if (require.main === module) {
--- a/script/test-deprecations.js
+++ b/script/test-deprecations.js
@ -30,9 +30,22 @@ If either check fails, the process exits with an error status (1).
  process.exit(0)
 }

-checkDeprecations(args)
+Promise.all([checkSelectorDeprecations(args), checkVariableDeprecations(args)]).then(
+  ([deprecationErrors, variableErrors]) => {
+    const errors = [...deprecationErrors, ...variableErrors]
+    if (errors.length) {
+      console.log(`\n${errors.length} error${errors.length === 1 ? '' : 's'}:`)
+      for (const error of errors) {
+        console.log(`${X} ${error}`)
+      }
+      process.exit(1)
+    } else {
+      console.log(`${V} no errors!`)
+    }
+  }
+)

-async function checkDeprecations(options = {}) {
+async function checkSelectorDeprecations(options = {}) {
  const {bundle = 'primer', version = 'latest'} = options

  const currentVersion = require('../package.json').version
@ -43,27 +56,30 @@ async function checkDeprecations(options = {}) {

  const {changed, added, removed} = diffLists(remote.selectors.values, local.selectors.values)
  if (changed === 0) {
-    console.log(`no selectors added or removed in bundle "${bundle}"`)
-    return
+    console.log(`${I} no selectors changed in bundle "${bundle}" (${version} -> ${currentVersion})`)
+    // return
  }

  const deprecations = versionDeprecations[currentVersion] || []
  const deprecatedSelectors = deprecations.reduce((list, deprecation) => list.concat(deprecation.selectors), [])
-  console.log(`${I} ${removed.length} selectors removed locally (compared with ${version})`)
-  console.log(`${I} ${deprecatedSelectors.length} selectors deprecated in v${currentVersion}`)
+  if (removed.length) {
+    console.log(`${I} ${removed.length} selectors removed locally (compared with ${version})`)
+  }
+  if (deprecatedSelectors.length) {
+    console.log(`${I} ${deprecatedSelectors.length} selectors to be deprecated in ${currentVersion}`)
+  }
  if (added.length) {
    console.log(`${I} ${added.length} selectors added`)
  }

  const errors = []
-  for (const deprecation of deprecations) {
-    for (const selector of deprecation.selectors) {
+  for (const {selectors = []} of deprecations) {
+    for (const selector of selectors) {
      if (!removed.includes(selector)) {
        const error = `"${selector}" deprecated, but not removed`
        errors.push(error)
-        console.log(`${X} ${error}`)
      } else {
-        console.log(`${V} "${selector}" is officially deprecated`)
+        console.log(`${V} selector "${selector}" is officially deprecated`)
      }
      deprecatedSelectors.push(selector)
    }
@ -73,15 +89,64 @@ async function checkDeprecations(options = {}) {
    if (!deprecatedSelectors.includes(removedSelector)) {
      const error = `"${removedSelector}" has been removed, but was not listed in versionDeprecations['${currentVersion}']`
      errors.push(error)
-      console.log(`${X} ${error}`)
    } else {
      // console.log(`${V} "${removedSelector}" removed and deprecated!`)
    }
  }

-  if (errors.length) {
-    process.exitCode = 1
+  return errors
+}
+
+async function checkVariableDeprecations(options = {}) {
+  const {version = 'latest'} = options
+
+  const currentVersion = require('../package.json').version
+  const varsPath = `dist/variables.json`
+
+  const local = require(`../${varsPath}`)
+  const remote = await fetch(`https://unpkg.com/@primer/css@${version}/${varsPath}`).then(res => res.json())
+
+  const {changed, added, removed} = diffLists(Object.keys(remote), Object.keys(local))
+  if (changed === 0) {
+    console.log(`${I} no variables changed (${version} -> ${currentVersion})`)
+    // return
  }
+
+  const deprecations = versionDeprecations[currentVersion] || []
+  const deprecatedVariables = deprecations.reduce((list, deprecation) => list.concat(deprecation.variables), [])
+  if (removed.length) {
+    console.log(`${I} ${removed.length} variables removed locally (compared with ${version})`)
+  }
+  if (deprecatedVariables.length) {
+    console.log(`${I} ${deprecatedVariables.length} variables to be deprecated in ${currentVersion}`)
+  }
+  if (added.length) {
+    console.log(`${I} ${added.length} variables added`)
+  }
+
+  const errors = []
+  for (const {variables = []} of deprecations) {
+    for (const variable of variables) {
+      if (!removed.includes(variable)) {
+        const error = `variable "${variable}" deprecated, but not removed`
+        errors.push(error)
+      } else {
+        console.log(`${V} variable "${variable}" is officially deprecated`)
+      }
+      deprecatedVariables.push(variable)
+    }
+  }
+
+  for (const removedVariable of removed) {
+    if (!deprecatedVariables.includes(removedVariable)) {
+      const error = `"${removedVariable}" has been removed, but was not listed in versionDeprecations['${currentVersion}']`
+      errors.push(error)
+    } else {
+      // console.log(`${V} "${removedVariable}" removed and deprecated!`)
+    }
+  }
+
+  return errors
 }

 function diffLists(before, after) {
--- a/src/navigation/underline-nav.scss
+++ b/src/navigation/underline-nav.scss
@ -32,7 +32,6 @@
    }
  }

-  &.selected, // TODO@14.0.0: remove &.selected
  &[role=tab][aria-selected=true],
  &[aria-current] {
    font-weight: $font-weight-bold;