Skip to content
This repository was archived by the owner on Sep 30, 2025. It is now read-only.

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented Mar 14, 2024

About 90% of the upgrade was handled by npx storybook@latest upgrade 🎉 (Feels like just yesterday we did the upgrade from v6 -> v7 😅)

Node v18

Storybook v8 requires node v18. But we're still stuck on node v16.

🚨 This PR is blocked by;

Performance

Calculated using hyperfine:

hyperfine --parameter-list branch main,storybook-8 --setup='git checkout {branch} && yarn && yarn build --filter=@shopify/polaris' --prepare='rm -rf polaris-react/build-internal/storybook/' --warmup 1 'yarn workspace @shopify/polaris run storybook:build --quiet'

main

Benchmark 1: yarn workspace @shopify/polaris run storybook:build --quiet
  Time (mean ± σ):     24.362 s ±  0.387 s    [User: 39.938 s, System: 4.076 s]
  Range (min … max):   23.896 s … 25.034 s    10 runs

storybook-8

Benchmark 2: yarn workspace @shopify/polaris run storybook:build --quiet
  Time (mean ± σ):     29.486 s ±  1.321 s    [User: 40.765 s, System: 3.868 s]
  Range (min … max):   28.419 s … 32.763 s    10 runs

Results

Summary
  yarn workspace @shopify/polaris run storybook:build --quiet (branch = main) ran
    1.21 ± 0.06 times faster than yarn workspace @shopify/polaris run storybook:build --quiet (branch = storybook-8)

Combined with the slow-down seen in the upgrade to v7, I'm starting to think Chromatic's marketing about better performance isn't true for us 😅

@jesstelford jesstelford changed the base branch from multiline-comments to main March 14, 2024 05:13
@jesstelford jesstelford added Tech Debt Key dependency Engineering Priority: Low Small, subtle details Blocked Something is preventing progress on this issue labels Mar 14, 2024
@lgriffee lgriffee added #gsd:38846 Admin Quality Improvements (Q1 2024) and removed #gsd:38846 Admin Quality Improvements (Q1 2024) labels Mar 15, 2024
@BPScott
Copy link
Member

BPScott commented Mar 16, 2024

I'm kicking this SB8 update over in checkout-web and found a storybook-a11y-test patch that works for SB8 (I've not tested if it falls back well in SB7/SB6) over in: https://github.com/Shopify/checkout-web/compare/sb8?expand=1

@jesstelford jesstelford marked this pull request as draft March 17, 2024 22:57
@github-actions github-actions bot added the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Apr 19, 2024
@github-actions github-actions bot removed the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Apr 22, 2024
@jesstelford jesstelford mentioned this pull request Apr 22, 2024
@jesstelford jesstelford changed the base branch from main to jest-29 April 22, 2024 12:47
@jesstelford jesstelford force-pushed the storybook-8 branch 2 times, most recently from 6c30c94 to b706003 Compare April 22, 2024 13:58
Base automatically changed from jest-29 to main April 23, 2024 00:26
jesstelford added a commit that referenced this pull request Apr 23, 2024
Good codebase hygeine.

But also, I need it so I can [upgrade to Storybook
v8](#11734) which requires me to
move to `@storybook/test-runner` which requires Jest 29.
@jesstelford jesstelford removed the Blocked Something is preventing progress on this issue label Apr 23, 2024
@jesstelford jesstelford force-pushed the storybook-8 branch 5 times, most recently from c5b469e to def7af2 Compare April 23, 2024 09:07
@@ -1,40 +0,0 @@
/* eslint-disable no-console */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced with polaris-react/.storybook/test-runner.ts

"storybook": "storybook dev -p ${POLARIS_STORYBOOK_PORT:=6006} --quiet",
"storybook:build": "storybook build -o build-internal/storybook/static"
"storybook:build": "storybook build -o build-internal/storybook/static",
"storybook:test": "concurrently -k -s first -n 'SB,TEST' -c 'magenta,blue' 'http-server build-internal/storybook/static --port 6006 --silent' 'wait-on tcp:6006 && test-storybook --maxWorkers=2 polaris-react/src/components/Modal/Modal.stories.tsx'"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command is from the Storybook docs

import styles from './DetailsPage.module.css';

export const DetailsPage = {
tags: ['skip-tests'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be in polaris-react/scripts/accessibility-check.js

<div style={{height: '500px'}}>
<Button onClick={handleChange}>Open</Button>
<Modal
instant
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prevents the accessibility checker from running while the CSS fade-in transition for Modal is still happening and giving false positives for color contrast issues.


export const Default = {
play: async ({canvasElement}) => {
await transitionsAllSettled(canvasElement);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prevents the accessibility checker from running while the CSS fade-in transition for Modal is still happening and giving false positives for color contrast issues.

@jesstelford jesstelford marked this pull request as ready for review April 23, 2024 09:13
@jesstelford jesstelford force-pushed the storybook-8 branch 2 times, most recently from e840d03 to d534e60 Compare April 23, 2024 10:40
@@ -0,0 +1,35 @@
/* eslint-env node */
Copy link
Contributor Author

@jesstelford jesstelford Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 🙇 Thankyou @BPScott for pointing me in the right direction with this one!
🤜 🤛

@jesstelford jesstelford merged commit 1fef062 into main Apr 23, 2024
@jesstelford jesstelford deleted the storybook-8 branch April 23, 2024 23:41
sophschneider pushed a commit that referenced this pull request Apr 24, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris@13.2.0

### Minor Changes

- [#11535](#11535)
[`bcd16df24`](bcd16df)
Thanks [@ShabanaRumane](https://github.com/ShabanaRumane)! - Added
support for setting `maxHeight` and `minHeight` on `Popover.Pane` and
`Combobox`


- [#11907](#11907)
[`45308c97a`](45308c9)
Thanks [@zakwarsame](https://github.com/zakwarsame)! - Added an optional
`fiterLabel` prop to `ActionList` to allow for a custom placeholder

### Patch Changes

- [#11897](#11897)
[`a83084b3b`](a83084b)
Thanks [@jesstelford](https://github.com/jesstelford)! - Fixed edges of
disabled `IndexTable.Row` `Checkbox` triggering selection


- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29


- [#11929](#11929)
[`9ee700be6`](9ee700b)
Thanks [@sophschneider](https://github.com/sophschneider)! - Rounded
`Navigation` at `mdDown` behind a feature flag


- [#11923](#11923)
[`ce13c4366`](ce13c43)
Thanks [@jesstelford](https://github.com/jesstelford)! - Update dev
dependency: `postcss-import@^15.1.0` -> `postcss-import@^16.1.0`


- [#11925](#11925)
[`364ada59e`](364ada5)
Thanks [@sophschneider](https://github.com/sophschneider)! - Updated
Frame to only apply rounded Frame when passed a `topBar`


- [#11734](#11734)
[`1fef06256`](1fef062)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to
Storybook v8


- [#11898](#11898)
[`1539f0e7c`](1539f0e)
Thanks [@jesstelford](https://github.com/jesstelford)! - Removed extra
padding around `IndexTable.Row` `Checkbox`


- [#11927](#11927)
[`5a32a3ff6`](5a32a3f)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
`prefers-reduced-motion` media queries to `Frame` width transitions


- [#11930](#11930)
[`b111629d7`](b111629)
Thanks [@jesstelford](https://github.com/jesstelford)! - Migrate
Storybook stories to CSF v3


- [#11805](#11805)
[`0a9b72721`](0a9b727)
Thanks [@LA1CH3](https://github.com/LA1CH3)! - Fixed `IndexTable`
`loading` prop to correctly show/hide loading UI when prop value changes

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-icons@9.0.1
    -   @shopify/polaris-tokens@9.0.1

## @shopify/polaris-icons@9.0.1

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29

## @shopify/polaris-migrator@1.0.1

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29


- [#11930](#11930)
[`b111629d7`](b111629)
Thanks [@jesstelford](https://github.com/jesstelford)! - Migrate
Storybook stories to CSF v3

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-tokens@9.0.1
    -   @shopify/stylelint-polaris@16.0.1

## @shopify/polaris-tokens@9.0.1

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29

## @shopify/stylelint-polaris@16.0.1

### Patch Changes

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-tokens@9.0.1

## polaris-for-vscode@1.0.1

### Patch Changes

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-tokens@9.0.1

## polaris.shopify.com@1.0.4

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29

- Updated dependencies
\[[`a83084b3b`](a83084b),
[`5ec70e688`](5ec70e6),
[`9ee700be6`](9ee700b),
[`bcd16df24`](bcd16df),
[`ce13c4366`](ce13c43),
[`364ada59e`](364ada5),
[`1fef06256`](1fef062),
[`45308c97a`](45308c9),
[`1539f0e7c`](1539f0e),
[`5a32a3ff6`](5a32a3f),
[`b111629d7`](b111629),
[`0a9b72721`](0a9b727)]:
    -   @shopify/polaris@13.2.0
    -   @shopify/polaris-icons@9.0.1
    -   @shopify/polaris-tokens@9.0.1

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants