Update storybook when SB7 regressions are fixed #1146
Comments
jattasNI
added
tech debt
triage
jattasNI
added a commit
that referenced
this issue
Mar 31, 2023
# Pull Request ## 🤨 Rationale We want to be on the latest storybook version because it fixes issues we've had with dependencies and it enables more powerful docs pages via mdx and Component Story Format enhancements. Notable changes: - landing page uses correct fonts and errors in console are gone (fixing #825 and #943 properly) - [no more Docs tab](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#70-docs-changes). Each story group has a docs page under it instead. - XD previews are gone (feature was deprecated) - Actions not working yet and some code previews have stopped working; see "Follow ups" below ## 👩💻 Implementation Following the [upgrade guide](https://storybook.js.org/docs/react/configure/upgrading#prereleases) didn't work very well because it is buggy with a monorepo. Fred's general strategy was to run the upgrade command from within `nimble-components` then delete the `node_modules` and `package-lock.json` that it creates there, then re-install everything from the root. The migration tool is responsible for the changes to: 1. storybook configuration files 2. package.json 3. renaming types like `Story` -> `StoryFn` and `storyName` to `name` 4. deleting the `withXD` decorator which is no longer supported Beyond that there were a few other manual changes: 1. establish static dependencies on components needed by each story. Storybook seems to have gotten more aggressive about compiling away unneeded components so if something isn't imported, it won't be found and components won't render correctly. This meant: 1. replacing a few literals for element tag names in stories with the `elementTag` export 2. updating the `nimble-menu` to explicitly depend on `nimble-anchored-region` (see comment on `menu/index.ts`) 2. Enable 'autodocs' for component and token stories but disable it for test stories 3. Add a patch file to work around storybookjs/storybook#21820 4. simplified return types of story generation functions in `storybook.ts` in response to storybook type changes 5. regenerated landing page screenshot and annotation now that docs tab is gone ## 🔜 Follow ups in future PRs Fixing these issues is tracked by #1146 . I tried upgrading to ts 4.7 but it didn't fix any of them. - [ ] storybookjs/storybook#21820 - [ ] Figure out why actions aren't firing when you click a button. Likely [need this decorator](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#removed-auto-injection-of-storybookaddon-actions-decorator) but it's not present in current rc: storybookjs/storybook#21308 - [ ] Some code previews (table, tooltip) are not available in the Docs page ## 🧪 Testing Chromatic caught several diffs which resulted in the manual changes described above 👍. ~~It's still showing new stories for the wafer map and button. (The wafer map one is caused by Chromatic getting confused when I tried renaming that story, ran a Chromatic build, and then reverted that change. I don't know the cause of the button change.)~~ <- this went away on a subsequent build Also manually spot checked many of the stories across browsers, especially ones that have workarounds in them like table and theme-aware tokens. ## ✅ Checklist <!--- Review the list and put an x in the boxes that apply or ~~strike through~~ around items that don't (along with an explanation). --> - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. --------- Co-authored-by: Fred Visser <1458528+fredvisser@users.noreply.github.com>
jattasNI
changed the title
jattasNI
added a commit
that referenced
this issue
Apr 4, 2023
# Pull Request ## 🤨 Rationale After #1134, the Actions tab in Storybook was no longer displaying events fired by components. This is one of the issues tracked in #1146. As described in the [Storybook 7 migration guide](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#removed-auto-injection-of-storybookaddon-actions-decorator), the actions addon is no longer enabled by default and it must be enabled by configuring the story with `decorators: [withActions]`. However, the `withActions` decorator wasn't exported from Storybook in a way that we could import it (see storybookjs/storybook#21887). A Storybook dev commented on that issue with a workaround that we can use until they fix that bug. ## 👩💻 Implementation 1. Add a [TypeScript shorthand ambient module declaration](https://www.typescriptlang.org/docs/handbook/modules.html#shorthand-ambient-modules) for the import URL that contains `withActions`. See `/utilities/tests/storybook-declarations.d.ts`. 2. Update all stories that expose an action to import `withActions` and configure it as a decorator (also reordered a few imports to group them by package) ## 🧪 Testing Manually verified many of the stories that have actions are now firing them. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
jattasNI
added a commit
that referenced
this issue
Apr 5, 2023
# Pull Request ## 🤨 Rationale Some of the storybook bug fixes in #1146 will likely rely on the [TypeScript 4.7 support for defining entry points in package.json](https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#package-json-exports-imports-and-self-referencing). While we can't uptake those fixes yet, I want to remove a potential blocker by moving us from TypeScript 4.6 to 4.7 ## 👩💻 Implementation 1. Replaced all `package.json` instances of `typescript": "~4.6.4"` with `typescript": "~4.7.4"`. That's the latest patch on the 4.7 minor release. 2. Ran `npm install` to regenerate `package-lock.json` We're not updating to TypeScript 4.8 or 5.0 yet because of blockers like #810. ## 🧪 Testing Ran `npm run pack` and brought the `nimble-angular` tgz into SystemLinkShared. Verified that it still builds and the example app runs even though that project is on TypeScript 4.6. ## ✅ Checklist <!--- Review the list and put an x in the boxes that apply or ~~strike through~~ around items that don't (along with an explanation). --> - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
rajsite
pushed a commit
that referenced
this issue
Apr 12, 2023
# Pull Request ## 🤨 Rationale Right after I submitted the upgrade to a release candidate of Storybook 7 in #1134, I noticed they [released the official 7.0.0](https://github.com/storybookjs/storybook/releases). Adopting the latest patch addresses some of the issues in #1146. ## 👩💻 Implementation 1. Update to latest non-RC build in package.json and regenerate package-lock.json 2. Regenerate one patch file since the file it was patching has a new name 3. Add another patch file since they introduced another instance of storybookjs/storybook#21820 4. Move off deprecated API for specifying `sourceTransform` as described in [migration guide](https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#source-block) 5. Delete `storybook-declarations.ts` since this bug was closed storybookjs/storybook#21887 ## 🧪 Testing Explored Storybook. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
1 task
rajsite
added a commit
that referenced
this issue
Jul 20, 2023
# Pull Request ## 🤨 Rationale Updates dependencies to the latest resolved by our package.json files without modifying those specifiers. Hoping it pre-emptively helps the discussion of [issues around dependency changes](#1335 (comment)) but useful regardless. As a bonus, fixes #1146 ## 👩💻 Implementation - Regenerate the lock file (delete lock file, node_modules, and npm install) - Update CI node and npm version to reflect Skyline recent update and better reflect local dev - Pin Blazor playwright version to correspond to latest released on nuget.org (as of writing [1.36.0](https://www.nuget.org/packages/Microsoft.Playwright)) vs latest released on npm which the previous specifier would resolve to (as of writing [1.36.1](https://www.npmjs.com/package/playwright)). - Update tests and stories based on lint rule behavior changes ## 🧪 Testing Manual test storybook build but rely on CI for most. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. --------- Co-authored-by: Malcolm Smith <20709258+msmithNI@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
🧹 Tech Debt
As part of #1134 we filed some issues to Storybook about workarounds we had to adopt and features we had to disable. This issue tracks the work to adopt the fixes once they're available.
patches/@storybook+html+7.0.0-rc.8.patchand delete the peer dependency on@storybook/html: *from the rootpackage.json. This is fixed in 7.0.8 but we can't uptake until this is fixed: [Bug]: @storybook/addon-actions: Type 'Addon_DecoratorFunction<unknown>' is not assignable to type 'DecoratorFunction<ReactRenderer, { storybookjs/storybook#22384storybook-declarations.d.tsadded in Fix Actions tab in Storybook #1154 once this issue is fixed: [Bug]: 7.0.0 - Module '"@storybook/addon-actions/decorator"' has no exported member 'withActions' storybookjs/storybook#21887configureActionsparameterdepthinpreview.jschanged meaning. We adapted in Fix Actions tab in Storybook #1154 but if they change it back we should revert that. [Bug]: configureActions depth parameter changed behavior between Storybook 6.5 and 7.0 storybookjs/storybook#21933The text was updated successfully, but these errors were encountered: