feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency madness ensues) #26907

rusackas · 2024-01-31T00:04:43Z

SUMMARY

This PR does the following:

Temporarily removes the deny-list for license types. For some reason it's blocking a goofily-licensed submodule, which is perfectly license compatible (MIT). I'll flip this back on once I'm done with this effort. Resolved.
Upgrades BOTH the Superset Storybook and superset-ui-demo to Storybook 7
Migrates all the Stories to use Controls instead of Knobs
Includes all the Stories from superset-ui-demo in the main Superset storybook (you can still run superset-ui-demo on its own as well, though)
Migrates mdx stories (overviews) to tsx - these might need touchups
Fixes a bunch of little random bugs along the way
Removes a bunch of package vulnerabilities (1 critical, 17 high, 11 moderate)
Bumps ESLint and Prettier by necessity, and for the sake of making this PR mergable, disables the new offences - we'll get those in follow-up PRs, I promise.
Touches many other moving parts - it's all a house of cards... but things seem to be running stably.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Run the root storybook and look at all the bells and whistles!

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

This reverts commit f969594.

codecov · 2024-02-14T20:02:26Z

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (af577d6) 67.16% compared to head (1019a4d) 67.16%.
Report is 2 commits behind head on master.

Files	Patch %	Lines
...end/src/visualizations/TimeTable/transformProps.ts	0.00%	4 Missing ⚠️
...ugin-chart-echarts/src/Waterfall/transformProps.ts	0.00%	3 Missing ⚠️
...-frontend/src/components/UiConfigContext/index.tsx	25.00%	3 Missing ⚠️
...rols/MetricControl/AdhocMetricEditPopoverTitle.tsx	86.95%	0 Missing and 3 partials ⚠️
...hart-pivot-table/src/react-pivottable/utilities.js	0.00%	2 Missing ⚠️
.../cell-renderers/ActionCell/ActionCell.overview.tsx	0.00%	2 Missing ⚠️
...res/alerts/components/AlertReportCronScheduler.tsx	87.50%	1 Missing and 1 partial ⚠️
...perset-ui-chart-controls/src/components/Select.tsx	0.00%	1 Missing ⚠️
...gin-chart-echarts/src/Timeseries/transformProps.ts	0.00%	0 Missing and 1 partial ⚠️
...d/components/DashboardBuilder/DashboardBuilder.tsx	66.66%	0 Missing and 1 partial ⚠️
... and 2 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #26907      +/-   ##
==========================================
- Coverage   67.16%   67.16%   -0.01%     
==========================================
  Files        1900     1901       +1     
  Lines       74447    74452       +5     
  Branches     8297     8301       +4     
==========================================
  Hits        50005    50005              
- Misses      22387    22392       +5     
  Partials     2055     2055

Flag	Coverage Δ
javascript	`56.84% <78.94%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.github/workflows/dependency-review.yml

eschutho

Wow! 🎉

mistercrunch · 2024-02-14T23:35:27Z

MERGE QUICK #yolo

rusackas · 2024-02-15T01:48:07Z

Just need a final code-owner check from @michael-s-molina @kgabryje or @geido (because I touched Select, MetadataBar, DropdownContainer) and we'll be good to smash this thing in!

geido

Thanks for all the great work that went into this! Let's keep an eye on Netlify to make sure everything goes smooth when we merge this up.

geido · 2024-02-15T10:31:29Z

superset-frontend/.eslintrc.js

@@ -161,6 +162,28 @@ module.exports = {
        'react/static-property-placement': 0, // re-enable up for discussion
        'prettier/prettier': 'error',
        'file-progress/activate': 1,
+        // delete me later: temporary rules to help with migration


Tracking this here https://github.com/orgs/apache/projects/315/views/1?pane=issue&itemId=53360236

geido · 2024-02-15T10:49:49Z

superset-frontend/src/components/Table/cell-renderers/ActionCell/ActionCell.overview.tsx

+ * under the License.
+ */
+
+// import { Meta, Source, Story, ArgsTable } from '@storybook/addon-docs';


Suggested change

// import { Meta, Source, Story, ArgsTable } from '@storybook/addon-docs';

geido · 2024-02-15T10:50:06Z

superset-frontend/src/components/Table/cell-renderers/ActionCell/ActionCell.overview.tsx

+// import { ActionMenuItem } from 'src/components/Table/cell-renderers/index';
+// import ActionCell from './index';


Suggested change

// import { ActionMenuItem } from 'src/components/Table/cell-renderers/index';

// import ActionCell from './index';

geido · 2024-02-15T10:56:01Z

superset-frontend/webpack.config.js

+      // {
+      //   test: /\.mdx?$/,
+      //   use: [
+      //     {
+      //       loader: require.resolve('@storybook/mdx2-csf/loader'),
+      //       options: {
+      //         skipCsf: false,
+      //         mdxCompileOptions: {
+      //           remarkPlugins: [remarkGfm],
+      //         },
+      //       },
+      //     },
+      //   ],
+      // },


Suggested change

// {

// test: /\.mdx?$/,

// use: [

// {

// loader: require.resolve('@storybook/mdx2-csf/loader'),

// options: {

// skipCsf: false,

// mdxCompileOptions: {

// remarkPlugins: [remarkGfm],

// },

// },

// },

// ],

// },

geido · 2024-02-15T10:57:01Z

superset-frontend/.eslintrc.js

@@ -310,6 +351,24 @@ module.exports = {
    'react/static-property-placement': 0, // disabled temporarily
    'react-prefer-function-component/react-prefer-function-component': 1,
    'prettier/prettier': 'error',
+    // disabling some things that come with the eslint 7->8 upgrade. Will address these in a separate PR


Tracked here as well https://github.com/orgs/apache/projects/315/views/1?pane=issue&itemId=53360236

kgabryje

Nothing to add on top of Diego’s comments, amazing work!!

michael-s-molina

Thank you for all the hard work @rusackas!

While testing the PR, I found the following problems:

Some components are not rendering anymore:

Some components are rendering but their content is not:

Some controls that used to work are not rendering. In this example, sizes and styles were a select:

Some plugins are overflowing the render area:

This list is not exhaustive, as I only tested a sample of the stories. I can confirm that these problems don't happen on master. Given the size of the PR and the possibility of conflicts, I'm ok with merging it as is and tackling the problems in a follow-up given that the Storybook is an isolated area.

Amazing work 👏🏼

michael-s-molina · 2024-02-14T22:01:26Z

superset-frontend/.eslintrc.js

@@ -161,6 +162,28 @@ module.exports = {
        'react/static-property-placement': 0, // re-enable up for discussion
        'prettier/prettier': 'error',
        'file-progress/activate': 1,
+        // delete me later: temporary rules to help with migration


Maybe add a TODO: prefix to make it discoverable by IDEs?

Suggested change

// delete me later: temporary rules to help with migration

// TODO: Delete me later. Temporary rules to help with migration.

michael-s-molina · 2024-02-14T22:01:48Z

superset-frontend/.eslintrc.js

@@ -203,6 +226,22 @@ module.exports = {
        ],
        'no-only-tests/no-only-tests': 'error',
        'max-classes-per-file': 0,
+        // temporary rules to help with migration - please re-enable!


Suggested change

// temporary rules to help with migration - please re-enable!

// TODO: Temporary rules to help with migration - please re-enable!

michael-s-molina · 2024-02-14T22:02:17Z

superset-frontend/.eslintrc.js

@@ -310,6 +351,24 @@ module.exports = {
    'react/static-property-placement': 0, // disabled temporarily
    'react-prefer-function-component/react-prefer-function-component': 1,
    'prettier/prettier': 'error',
+    // disabling some things that come with the eslint 7->8 upgrade. Will address these in a separate PR


Suggested change

// disabling some things that come with the eslint 7->8 upgrade. Will address these in a separate PR

// TODO: Disabling some things that come with the eslint 7->8 upgrade. Will address these in a separate PR.

rusackas · 2024-02-15T14:51:48Z

Follow-up PRs to address all of the above coming soon! I was spot checking stories as I ported them to the new format, but knew that some would get screwed up along the way. Let the polishing phase begin!

geido · 2024-02-16T17:17:05Z

Thank you for all the hard work @rusackas!

While testing the PR, I found the following problems:

Some components are not rendering anymore:

Some components are rendering but their content is not:
Some controls that used to work are not rendering. In this example, sizes and styles were a select:
Some plugins are overflowing the render area:
This list is not exhaustive, as I only tested a sample of the stories. I can confirm that these problems don't happen on master. Given the size of the PR and the possibility of conflicts, I'm ok with merging it as is and tackling the problems in a follow-up given that the Storybook is an isolated area.

Amazing work 👏🏼

Tracking this here https://github.com/orgs/apache/projects/315/views/1?pane=issue&itemId=53539080

…-to-the-embedded-dashboard * master: (1182 commits) fix(ci): mypy pre-commit issues (apache#27161) feat(Alerts and Reports): Modal redesign (apache#26202) refactor: Migrate ErrorBoundary to typescript (apache#27143) chore(tests): Remove unnecessary explicit Flask-SQLAlchemy session expunges (apache#27136) fix(plugins): Apply dashboard filters to comparison query in BigNumber with Time Comparison chart (apache#27138) fix: Duplicated toast messages (apache#27135) docs: add Geotab to users list (apache#27134) fix: Plain error message when visiting a dashboard via permalink without permissions (apache#27132) fix: ID param for DELETE ssh_tunnel endpoint (apache#27130) chore(hail mary): Update package-lock.json via npm-audit-fix (apache#26693) chore: lower cryptography min version to 41.0.2 (apache#27129) docs(miscellaneous): Export Datasoruces: export datasources exports to ZIP (apache#27120) fix(pivot-table-v2): Added forgotten translation pivot table v2 (apache#22840) fix: RLS modal overflow (apache#27128) refactor: Updates some database columns to MediumText (apache#27119) fix: gevent upgrade to 23.9.1 (apache#27112) fix: removes old deprecated sqllab endpoints (apache#27117) feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency madness ensues) (apache#26907) fix: bump grpcio, urllib3 and paramiko (apache#27124) chore(internet_port): added new ports and removed unnecessary string class (apache#27078) ...

… madness ensues) (apache#26907)

pull-request-size bot added the size/XXL label Jan 31, 2024

github-actions bot added the dependencies:npm label Jan 31, 2024

rusackas marked this pull request as draft January 31, 2024 00:06

pull-request-size bot added size/L and removed size/XXL labels Feb 2, 2024

rusackas force-pushed the upgrade-to-storybook-7 branch from c890960 to c72ef0d Compare February 2, 2024 22:33

rusackas changed the title ~~chore(storybook): Upgrading to storybook 7~~ feat(storybook): Co-habitating/Upgrading Storybooks to 7 Feb 2, 2024

pull-request-size bot added size/XXL size/XL and removed size/L size/XXL size/XL labels Feb 3, 2024

github-actions bot added plugins github_actions Pull requests that update GitHub Actions code packages labels Feb 8, 2024

rusackas added 14 commits February 8, 2024 14:53

chore(storybook): Upgrading to storybook 7

5430bbf

lots of story tweaks... these might not work ¯\_(ツ)_/¯

ee76eaa

Revert "lots of story tweaks... these might not work ¯\_(ツ)_/¯"

135c9d9

This reverts commit f969594.

core stories updated to remove knobs

ef6e69d

turning autodocs off... that sure doesn't work.

8564de2

7.6.12... I mean why not?!

df1da2d

removing duplicate addon

2f16a34

New MDX loader... working better!

1011f0b

bumping remark-gfm

6ab7e2a

renaming story so it gets picked up properly by storybook

79b3a43

Migrating stuff from the monorepo

3e04314

fixing path import for SB7 and allowing progressive migration of Stories

df230d5

Fixing story sorting for SB7

3c3cdaa

oops, missed a file... part of the superset-ui-demo bump to 7.6.12

9be4ec0

rusackas and others added 3 commits February 14, 2024 11:01

restoring file from master

887473e

Merge branch 'master' into upgrade-to-storybook-7

e5b2a6c

fixing problematic dependencies

c462bff

rusackas marked this pull request as ready for review February 14, 2024 19:38

mistercrunch reviewed Feb 14, 2024

View reviewed changes

.github/workflows/dependency-review.yml Show resolved Hide resolved

restoring license deny list

3a1b55a

github-actions bot removed the github_actions Pull requests that update GitHub Actions code label Feb 14, 2024

adding [email protected] licensing exception

7c05e91

github-actions bot added the github_actions Pull requests that update GitHub Actions code label Feb 14, 2024

comment about license

1019a4d

mistercrunch approved these changes Feb 14, 2024

View reviewed changes

eschutho approved these changes Feb 14, 2024

View reviewed changes

rusackas changed the title ~~feat(storybook): Co-habitating/Upgrading Storybooks to 7~~ feat(storybook): Co-habitating/Upgrading Storybooks to v7, and dependency madness Feb 14, 2024

rusackas changed the title ~~feat(storybook): Co-habitating/Upgrading Storybooks to v7, and dependency madness~~ feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency madness ensues) Feb 14, 2024

geido approved these changes Feb 15, 2024

View reviewed changes

kgabryje approved these changes Feb 15, 2024

View reviewed changes

michael-s-molina reviewed Feb 15, 2024

View reviewed changes

rusackas merged commit 753ef69 into master Feb 15, 2024
39 checks passed

rusackas deleted the upgrade-to-storybook-7 branch February 15, 2024 15:57

sfirke pushed a commit to sfirke/superset that referenced this pull request Mar 22, 2024

feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency…

be8d01b

… madness ensues) (apache#26907)

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 4.0.0 labels Apr 17, 2024

vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024

feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency…

bc474ed

… madness ensues) (apache#26907)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency madness ensues) #26907

feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency madness ensues) #26907

rusackas commented Jan 31, 2024 •

edited

Loading

codecov bot commented Feb 14, 2024 •

edited

Loading

eschutho left a comment

mistercrunch commented Feb 14, 2024

rusackas commented Feb 15, 2024

geido left a comment

geido Feb 15, 2024

geido Feb 15, 2024

geido Feb 15, 2024

geido Feb 15, 2024

geido Feb 15, 2024

kgabryje left a comment

michael-s-molina left a comment •

edited

Loading

michael-s-molina Feb 14, 2024

michael-s-molina Feb 14, 2024

michael-s-molina Feb 14, 2024

rusackas commented Feb 15, 2024

geido commented Feb 16, 2024

		// import { ActionMenuItem } from 'src/components/Table/cell-renderers/index';
		// import ActionCell from './index';

	// delete me later: temporary rules to help with migration
	// TODO: Delete me later. Temporary rules to help with migration.

	// temporary rules to help with migration - please re-enable!
	// TODO: Temporary rules to help with migration - please re-enable!

	// disabling some things that come with the eslint 7->8 upgrade. Will address these in a separate PR
	// TODO: Disabling some things that come with the eslint 7->8 upgrade. Will address these in a separate PR.

feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency madness ensues) #26907

feat(storybook): Co-habitating/Upgrading Storybooks to v7 (dependency madness ensues) #26907

Conversation

rusackas commented Jan 31, 2024 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Feb 14, 2024 • edited Loading

Codecov Report

eschutho left a comment

Choose a reason for hiding this comment

mistercrunch commented Feb 14, 2024

rusackas commented Feb 15, 2024

geido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kgabryje left a comment

Choose a reason for hiding this comment

michael-s-molina left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rusackas commented Feb 15, 2024

geido commented Feb 16, 2024

rusackas commented Jan 31, 2024 •

edited

Loading

codecov bot commented Feb 14, 2024 •

edited

Loading

michael-s-molina left a comment •

edited

Loading