Skip to content

Commit

Permalink
🏗♻️🚮 Disable @storybook/addon-knobs (#37681)
Browse files Browse the repository at this point in the history
Related to #35923

`@storybook/addon-knobs` has been deprecated, and it's officially obsolete. This prevents us from upgrading the Storybook version due to a peer dependency conflict. 

This change:

- Removes `@storybook/addon-knobs`
- Disables Storybook files that require Knobs. We use the forbidden term allowlist to find disabled files. 
- Since `amp-animation` tests do not match the allowlist schema for disabling, we migrate those to Controls instead.
  • Loading branch information
alanorozco committed Feb 15, 2022
1 parent 6e7cb14 commit 1e82a4d
Show file tree
Hide file tree
Showing 11 changed files with 111 additions and 881 deletions.
9 changes: 4 additions & 5 deletions build-system/tasks/storybook/env/amp/main.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
const {globExcludeDisabledStorybookFiles} = require('../disabled-stories');

const rootDir = '../../../../..';

module.exports = {
staticDirs: [rootDir],
stories: [
stories: globExcludeDisabledStorybookFiles([
`${rootDir}/src/builtins/storybook/*.amp.js`,
`${rootDir}/extensions/**/*.*/storybook/*.amp.js`,
],
]),
addons: [
// TODO(alanorozco): AMP previews are loaded inside an iframe, so the a11y
// addon is not able to inspect the tree inside it. Its results are incorrect,
Expand All @@ -14,9 +16,6 @@ module.exports = {
// '@storybook/addon-a11y',
'@storybook/addon-viewport/register',
'@storybook/addon-controls/register',
// TODO(#35923): Remove addon-knobs once all stories are migrated to
// addon-controls (args/argTypes).
'@storybook/addon-knobs',
'@ampproject/storybook-addon',
],
webpackFinal: (config) => {
Expand Down
8 changes: 0 additions & 8 deletions build-system/tasks/storybook/env/amp/register.js

This file was deleted.

46 changes: 46 additions & 0 deletions build-system/tasks/storybook/env/disabled-stories.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const globby = require('globby');
const {yellow} = require('kleur/colors');
const {logWithoutTimestamp} = require('../../../common/logging');

/**
* Remove disabled Storybook files from an evaluated glob pattern.
* This is required since the disabled Stories use the Knobs addon, which is now
* obsolete and prevents us from upgrading the Storybook version.
* @param {string[]} inclusionPattern
* @return {string[]}
*/
function globExcludeDisabledStorybookFiles(inclusionPattern) {
const {
forbiddenTermsGlobal,
} = require('../../../test-configs/forbidden-terms');
const forbiddenTerm = `@storybook/${''}addon-knobs`;
const forbiddenTermsEntry = forbiddenTermsGlobal[forbiddenTerm];
if (!forbiddenTermsEntry?.allowlist?.length) {
throw new Error(
`Forbidden terms entry for "${forbiddenTerm}" not found, or it lacks an allowlist.` +
'\nThis likely means that globExcludeDisabledStorybookFiles() should be removed.' +
'\nIts callsites can be replaced with its argument, like the diff at:' +
'\nhttps://gist.github.com/alanorozco/6e8a64a38cb0d6967c193784624d1011'
);
}
const excluded = new Set(forbiddenTermsEntry.allowlist);
const prefiltered = globby.sync(inclusionPattern);
const filtered = prefiltered.filter((filename) => {
const relativeToRoot = filename.replace(/^([.]{1,2}\/)+/, '');
return !excluded.has(relativeToRoot);
});
const excludedTotal = prefiltered.length - filtered.length;
if (excludedTotal > 0) {
logWithoutTimestamp(
yellow(
`WARNING: ${excludedTotal} Storybook files have been disabled due to usage of "${forbiddenTerm}"` +
`\nSee allowlist for "${forbiddenTerm}" in build-system/test-configs/forbidden-terms.js`
)
);
}
return filtered;
}

module.exports = {
globExcludeDisabledStorybookFiles,
};
9 changes: 4 additions & 5 deletions build-system/tasks/storybook/env/preact/main.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
const {globExcludeDisabledStorybookFiles} = require('../disabled-stories');

const rootDir = '../../../../..';

module.exports = {
staticDirs: [rootDir],
stories: [
stories: globExcludeDisabledStorybookFiles([
`${rootDir}/src/**/storybook/!(*.amp).js`,
`${rootDir}/extensions/**/*.*/storybook/!(*.amp).js`,
],
]),
addons: [
'@storybook/addon-a11y',
'@storybook/addon-viewport/register',
'@storybook/addon-controls/register',
// TODO(#35923): Remove addon-knobs once all stories are migrated to
// addon-controls (args/argTypes).
'@storybook/addon-knobs',
],
webpackFinal: async (config) => {
// Disable entry point size warnings.
Expand Down
9 changes: 4 additions & 5 deletions build-system/tasks/storybook/env/react/main.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
const {globExcludeDisabledStorybookFiles} = require('../disabled-stories');

const rootDir = '../../../../..';

module.exports = {
staticDirs: [rootDir],
// Unlike the `amp` and `preact` environments, we search Storybook files only
// under component paths. This is because only components have React build
// output, but directories in src/ outside src/bento/components/ do not.
stories: [
stories: globExcludeDisabledStorybookFiles([
`${rootDir}/extensions/**/*.*/storybook/!(*.amp).js`,
`${rootDir}/src/bento/components/**/*.*/storybook/*.js`,
],
]),
addons: [
'@storybook/addon-a11y',
'@storybook/addon-viewport/register',
'@storybook/addon-controls/register',
// TODO(#35923): Remove addon-knobs once all stories are migrated to
// addon-controls (args/argTypes).
'@storybook/addon-knobs',
],
webpackFinal: (config) => {
// Disable entry point size warnings.
Expand Down
Loading

0 comments on commit 1e82a4d

Please sign in to comment.