Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove storybook doc references #8944

Merged
merged 7 commits into from
Mar 19, 2024

Conversation

brandentheintern
Copy link
Contributor

Related Issue: #8911

Summary

Storybook is no longer the central place for documentation for calcite. Thus, there are some parts of the codebase that must be refactored to better reflect that change.

@github-actions github-actions bot added the chore Issues with changes that don't modify src or test files. label Mar 15, 2024
@brandentheintern brandentheintern changed the title Chore: deprecate storybook doc references Refactor: deprecate storybook doc references Mar 15, 2024
@brandentheintern brandentheintern changed the title Refactor: deprecate storybook doc references chore: deprecate storybook doc references Mar 15, 2024
@jcfranco jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Mar 15, 2024
"start": "npm run util:clean-js-files && concurrently --kill-others --raw \"tsc --project ./tsconfig-demos.json --watch\" \"npm run build:watch-dev -- --serve\"",
"test": "stencil test --no-docs --no-build --spec --e2e",
"test:prerender": "npm run build -- --prerender",
"test:watch": "npm run build && npm run test -- -- --watchAll",
"util:build-docs": "npm run util:prep-build-reqs && stencil build --docs --config stencil.storybook.config.ts && npm run lint:md",
"util:prep-storybook-build": "npm run util:prep-build-reqs && stencil build --docs --config stencil.storybook.config.ts && npm run lint:md",
Copy link
Member

Choose a reason for hiding this comment

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

Let's reorder all renamed scripts so that they remain in alphabetical order. This makes it easier to find when scanning the code.

@@ -26,21 +26,21 @@
"build-storybook": "npm run util:build-docs && NODE_OPTIONS=--openssl-legacy-provider build-storybook --output-dir ./docs --quiet",
"clean": "npm run util:clean-js-files && npm run util:clean-readmes && rimraf node_modules dist www hydrate docs .turbo",
"deps:update": "updtr --exclude chalk cheerio typescript @types/jest jest jest-cli ts-jest @whitespace/storybook-addon-html && npm audit fix",
"docs": "build-storybook",
"docs:preview": "npm run util:build-docs && NODE_OPTIONS=--openssl-legacy-provider STORYBOOK_SCREENSHOT_LOCAL_BUILD=true start-storybook",
"screenshot-test": "build-storybook",
Copy link
Member

Choose a reason for hiding this comment

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

This name reads as if it will run the screenshot tests. How about screenshot-tests and screenshot-tests:preview?

"lint": "concurrently npm:lint:*",
"lint:html": "prettier --write \"**/*.html\" >/dev/null",
"lint:json": "prettier --write \"**/*.json\" >/dev/null",
"lint:md": "prettier --write \"**/*.md\" >/dev/null && markdownlint \"**/*.md\" --fix --dot --ignore-path .gitignore",
"lint:scss": "stylelint --fix \"src/**/*.scss\" && prettier --write \"**/*.scss\" >/dev/null",
"lint:ts": "eslint --ext .ts,.tsx --fix . && prettier --write \"**/*.ts?(x)\" >/dev/null",
"posttest": "npm run test:prerender",
"release:docs": "npm run docs && storybook-to-ghpages --existing-output-dir=docs",
"release:docs": "npm run screenshot-test && storybook-to-ghpages --existing-output-dir=docs",
Copy link
Member

Choose a reason for hiding this comment

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

Missed this in our chat yesterday, but we should also rename this script as well. It publishes storybook screenshot tests to GitHub pages. Maybe screenshot-tests:publish?

"docs": "build-storybook",
"docs:preview": "npm run util:build-docs && NODE_OPTIONS=--openssl-legacy-provider STORYBOOK_SCREENSHOT_LOCAL_BUILD=true start-storybook",
"screenshot-test": "build-storybook",
"screenshot-test-preview": "npm run util:build-docs && NODE_OPTIONS=--openssl-legacy-provider STORYBOOK_SCREENSHOT_LOCAL_BUILD=true start-storybook",
Copy link
Member

Choose a reason for hiding this comment

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

Make sure to test changes to scripts. This is still referencing util:build-docs and is causing an issue with Chromatic builds:

Chromatic CLI v***.0.8
https://www.chromatic.com/docs/cli

Authenticating with Chromatic
    → Connecting to https://index.chromatic.com
Authenticated with Chromatic
    → Using project token '********8823'
Retrieving git information
Retrieved git information
    → Commit '4e8433***' on branch 'brandentheintern/89***-remove-docs-tab'; found *** parent build
Collecting Storybook metadata
No viewlayer package listed in dependencies. Checking transitive dependencies.
Collected Storybook metadata
    → ; no supported addons found
Initializing build
Initialized build
    → Build 4093 initialized
Building your Storybook
    → Running command: npm run build-storybook -- --output-dir /tmp/chromatic--2***42-dkKTNMFhIvej
    → [*                   ]
The CLI tried to run your build-storybook script, but the command failed. This indicates a problem with your Storybook. Here's what to do:

- Check the Storybook build log printed below.
- Run npm run build-storybook or yarn build-storybook yourself and make sure it outputs a valid Storybook by opening the generated index.html in your browser.
- Review the build-storybook CLI options at https://storybook.js.org/docs/configurations/cli-options/#for-build-storybook

Command failed with exit code ***: npm run build-storybook -- --output-dir /tmp/chromatic--2***42-dkKTNMFhIvej
npm ERR! Lifecycle script `util:build-docs` failed with error: 
npm ERR! Error: Missing script: "util:build-docs"

To see a list of scripts, run:
  npm run 
npm ERR!   in workspace: @esri/calcite-components@2.7.0-next.4 
npm ERR!   at location: /home/runner/work/calcite-design-system/calcite-design-system/packages/calcite-components 
npm ERR! Lifecycle script `build-storybook` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: @esri/calcite-components@2.7.0-next.4 
npm ERR!   at location: /home/runner/work/calcite-design-system/calcite-design-system/packages/calcite-components 

ℹ Build command:
npm run build-storybook -- --output-dir /tmp/chromatic--2***42-dkKTNMFhIvej

@jcfranco
Copy link
Member

@brandentheintern Can you also revisit the PR title to better convey the changes? It suggests deprecation, which isn't accurate.

@brandentheintern brandentheintern changed the title chore: deprecate storybook doc references chore: remove storybook doc references Mar 15, 2024
@jcfranco jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Mar 15, 2024
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

One small tweak and this will be good to merge. 🚀

"release:docs": "npm run screenshot-test && storybook-to-ghpages --existing-output-dir=docs",
"screenshot-tests": "build-storybook",
"screenshot-tests:publish": "npm run screenshot-tests && storybook-to-ghpages --existing-output-dir=docs",
"screenshot-tests:preview": "npm run util:prep-storybook-build && NODE_OPTIONS=--openssl-legacy-provider STORYBOOK_SCREENSHOT_LOCAL_BUILD=true start-storybook",
Copy link
Member

Choose a reason for hiding this comment

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

preview should go before publish.

@jcfranco jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Mar 18, 2024
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

🚀

@brandentheintern brandentheintern merged commit 06843f7 into main Mar 19, 2024
16 checks passed
@brandentheintern brandentheintern deleted the brandentheintern/8911-remove-docs-tab branch March 19, 2024 19:07
@github-actions github-actions bot added this to the 2024-03-26 - Mar Release milestone Mar 19, 2024
jcfranco added a commit that referenced this pull request Apr 12, 2024
**Related Issue:** N/A

## Summary

This is no longer be needed after recent Storybook updates (see
#8944 and
#9030).

✨🧹✨
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issues with changes that don't modify src or test files. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants