Storybook: upgrade to v6, Styled-Components: upgrade to v5 (includes brave-ui) #8120

petemill · 2021-03-04T11:55:01Z

Fix brave/brave-browser#14305 (npm audit issues), as well as brings some much-needed storybook bug fixes.

Here's a find/replace regex I used for the styled-components typescript format: https://regex101.com/r/5X75t3/1/

core-js added as a dependency due to storybookjs/storybook#11255

Requires:

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

npm run storybook and verify each item is showing and with correct styles.
Build browser and run as many WebUI as possible. Check styles and theming.

.storybook/webpack.config.js

components/brave_extension/extension/brave_extension/stories/story.tsx

.storybook/preview.js

zenparsing · 2021-03-10T03:17:30Z

package.json

@@ -314,6 +311,7 @@
    "array-move": "^2.2.1",
    "bignumber.js": "^7.2.1",
    "bluebird": "^3.5.1",
+    "core-js": "^3.9.1",


Where does the core-js dep come from?

To fix storybookjs/storybook#11255. Apologies for not explicitly mentioning. Maybe I should have added a comment, but not sure how npm handles that.

zenparsing · 2021-03-10T03:19:35Z

components/webcompat_reporter/ui/components/basic/structure.ts

 import Button, { Props as ButtonProps } from 'brave-ui/components/buttonsIndicators/button'

-export const SideBySideButtons = styled<{}, 'div'>('div')`
+export const SideBySideButtons = styled('div')<{}>`


Is the empty props object needed, or can it just be styled('div') (or even styled.div)?

It can, yes, and it will still get correctly-typed theme props. But we can do it in a follow-up. This was the result of a find/replace just switching the order and removing the duplication.

zenparsing · 2021-03-10T03:20:08Z

components/webcompat_reporter/ui/components/basic/display.ts

@@ -3,10 +3,11 @@
 * License, v. 2.0. If a copy of the MPL was not distributed with this file,
 * You can obtain one at http://mozilla.org/MPL/2.0/. */

-import styled from 'brave-ui/theme'
+import styled from 'styled-components'


This dependency flattening is great - thanks!

zenparsing · 2021-03-10T03:21:57Z

components/common/StorybookThemeProvider.tsx

@@ -0,0 +1,37 @@
+// Copyright (c) 2020 The Brave Authors. All rights reserved.


Eternally March 2020 😛

In my defense, I did write this code in 2020 when I started this branch!

zenparsing · 2021-03-10T03:23:27Z

components/brave_rewards/resources/tsconfig.json

@@ -0,0 +1,9 @@
+{
+  "extends": "../../../tsconfig",
+  "include": [


Curious: what does the addition of this file do?

It was the only way I could see to get correct intellisense in Visual Studio Code working for certain types (I think theme properties, but I'm having trouble remembering now). Perhaps the project was otherwise too big unless localized to the specific dir the current file open is in. So, a different tsconfig per WebUI "project" seems appropriate anyway.

zenparsing · 2021-03-10T03:24:31Z

components/brave_welcome_ui/theme/index.ts

+
+type Theme = IBraveTheme & IWelcomeTheme
+
+export default styled as unknown as Styled.ThemedStyledInterface<Theme>


Is the as unknown as ... intentional?

yes, it allows the type to be cast, otherwise you cannot directly cast styled to include the theme. But we know that it does include the specific Welcome theme, so it's ok.

This is the suggested way to have multiple themes in a project. Otherwise it will just assume our re-defined DefaultTheme.

zenparsing · 2021-03-10T03:32:58Z

components/brave_rewards/resources/ui/components/grantCaptcha/index.tsx

@@ -97,10 +97,10 @@ export default class GrantCaptcha extends React.PureComponent<Props, {}> {
    return (
      <StyledWrapper
        id={id}
-        innerRef={this.refWrapper}
+        ref={this.refWrapper}


components/brave_rewards/resources/ui/components/checkout/dialogTitle/style.ts

zenparsing · 2021-03-10T03:41:09Z

third_party/npm_core-js/README.chromium

@@ -0,0 +1,4 @@
+Name: core-js


What is this file for?

all of our deps in package.json must define the license file, which I believe gets automatically included in chrome://credits. Otherwise the build fails.

diracdeltas · 2021-03-10T04:11:06Z

npm run tslint shows a few errors on this branch. dep updates lgtm though.

components/brave_rewards/resources/ui/components/rewardsButton/style.ts

components/brave_rewards/resources/ui/components/tableDonation/style.ts

bsclifton

Left some comments - but looks great! 😄 This is a whole lot of work, thanks for taking it on!

petemill · 2021-03-10T12:00:19Z

Restarted Windows build, it had typescript errors even though all other platforms did not 🤷

petemill · 2021-03-11T00:14:17Z

Windows CI still giving off errors that were previously fixed within the brave-ui PR that this pulls in via package.json, checking...

petemill · 2021-03-11T08:25:35Z

skipping all other platforms for CI apart from Windows whilst I attempt to fix it

core-js added to fix a build reference issue

…uired with the no-longer-maintained @dump247/storybook-state package Ran `npm update react-json-view --depth 2` to give this change

…dules (and same context singletons)

(only CI fixes)

…e.json)

petemill · 2021-03-11T21:16:14Z

CI now passed on all platforms 🍾

petemill self-assigned this Mar 4, 2021

petemill mentioned this pull request Mar 5, 2021

Upgrade styled-components to v5 #8136

Closed

petemill force-pushed the storybook-v6-2 branch 2 times, most recently from 59b55e2 to 72b1c95 Compare March 9, 2021 22:26

petemill requested review from zenparsing, simonhong, antonok-edm, bsclifton and diracdeltas March 9, 2021 22:28

petemill changed the title ~~Storybook: upgrade to v6~~ Storybook: upgrade to v6, Styled-Components: upgrade to v5 Mar 9, 2021

petemill changed the title ~~Storybook: upgrade to v6, Styled-Components: upgrade to v5~~ Storybook: upgrade to v6, Styled-Components: upgrade to v5 (includes brave-ui) Mar 9, 2021

bsclifton reviewed Mar 9, 2021

View reviewed changes

.storybook/webpack.config.js Outdated Show resolved Hide resolved

diracdeltas reviewed Mar 9, 2021

View reviewed changes

components/brave_extension/extension/brave_extension/stories/story.tsx Outdated Show resolved Hide resolved

diracdeltas reviewed Mar 9, 2021

View reviewed changes

.storybook/preview.js Outdated Show resolved Hide resolved

zenparsing reviewed Mar 10, 2021

View reviewed changes

petemill force-pushed the storybook-v6-2 branch 2 times, most recently from 8cc477f to b56c8e7 Compare March 10, 2021 04:54

bsclifton reviewed Mar 10, 2021

View reviewed changes

components/brave_rewards/resources/ui/components/rewardsButton/style.ts Show resolved Hide resolved

bsclifton reviewed Mar 10, 2021

View reviewed changes

components/brave_rewards/resources/ui/components/tableDonation/style.ts Show resolved Hide resolved

bsclifton reviewed Mar 10, 2021

View reviewed changes

petemill force-pushed the storybook-v6-2 branch 2 times, most recently from 21c0b2d to 8c0357b Compare March 10, 2021 09:58

bsclifton approved these changes Mar 10, 2021

View reviewed changes

petemill added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Mar 11, 2021

petemill added 9 commits March 11, 2021 21:31

Storybook: upgrade to v6

405cc59

core-js added to fix a build reference issue

npm audit: fix low severity vulnerability with node-fetch that is req…

25f206e

…uired with the no-longer-maintained @dump247/storybook-state package Ran `npm update react-json-view --depth 2` to give this change

Upgrade to styled-components v5

cfb39fe

Upgrade to styled-components v5 (more required manual changes)

4659f94

Ensure that brave-ui and brave-core use the same styled-components mo…

698addf

…dules (and same context singletons)

point brave-ui at latest master (no changes)

e05083d

(only CI fixes)

fix elliptic npm audit moderate issues

f99c2a7

point brave-ui to include brave/brave-ui#579

d404f23

bump brave-ui version (no changes apart from version number in packag…

4244de3

…e.json)

petemill force-pushed the storybook-v6-2 branch from 8c0357b to 4244de3 Compare March 11, 2021 08:34

petemill removed CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Mar 11, 2021

petemill merged commit 401adb5 into master Mar 11, 2021

petemill deleted the storybook-v6-2 branch March 11, 2021 21:17

petemill added this to the 1.23.x - Nightly milestone Mar 11, 2021

This was referenced Mar 18, 2021

Site-icon's focus ring is too tall, and is clipped beneath the site icon below it brave/brave-browser#14802

Closed

[Desktop] Customize Dashboard modal window heading text not visible in dark theme - follow up to 14754 brave/brave-browser#14785

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storybook: upgrade to v6, Styled-Components: upgrade to v5 (includes brave-ui) #8120

Storybook: upgrade to v6, Styled-Components: upgrade to v5 (includes brave-ui) #8120

petemill commented Mar 4, 2021 •

edited

Loading

zenparsing Mar 10, 2021

petemill Mar 10, 2021

zenparsing Mar 10, 2021

petemill Mar 10, 2021

zenparsing Mar 10, 2021

zenparsing Mar 10, 2021

bsclifton Mar 10, 2021

petemill Mar 10, 2021

zenparsing Mar 10, 2021

petemill Mar 10, 2021 •

edited

Loading

zenparsing Mar 10, 2021

petemill Mar 10, 2021

zenparsing Mar 10, 2021

zenparsing Mar 10, 2021

petemill Mar 10, 2021

diracdeltas commented Mar 10, 2021

bsclifton left a comment

petemill commented Mar 10, 2021

petemill commented Mar 11, 2021

petemill commented Mar 11, 2021

petemill commented Mar 11, 2021

		@@ -0,0 +1,37 @@
		// Copyright (c) 2020 The Brave Authors. All rights reserved.


		type Theme = IBraveTheme & IWelcomeTheme

		export default styled as unknown as Styled.ThemedStyledInterface<Theme>

Storybook: upgrade to v6, Styled-Components: upgrade to v5 (includes brave-ui) #8120

Storybook: upgrade to v6, Styled-Components: upgrade to v5 (includes brave-ui) #8120

Conversation

petemill commented Mar 4, 2021 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petemill Mar 10, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diracdeltas commented Mar 10, 2021

bsclifton left a comment

Choose a reason for hiding this comment

petemill commented Mar 10, 2021

petemill commented Mar 11, 2021

petemill commented Mar 11, 2021

petemill commented Mar 11, 2021

petemill commented Mar 4, 2021 •

edited

Loading

petemill Mar 10, 2021 •

edited

Loading