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

Angular: Fix sourceDecorator to apply excludeDecorators flag #29069

Merged

Conversation

JSMike
Copy link
Contributor

@JSMike JSMike commented Sep 7, 2024

closes #24560, closes #25147, closes #23659, closes #12022

What I did

Added check to see if paramaters.docs.source.excludeDecorators was set and if so return originalStoryFn template

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

  1. Run angular sandbox for template, e.g. yarn task --task dev --template angular-cli/default-ts --prod --start-from=compile
  2. Open Storybook in your browser
  3. Go to Button/Docs
  4. Open editor to sandbox/angular-cli-default-ts/src/stories/button.stories.ts
  5. add componentWrapperDecorator to the imports on line 1 from @storybook/angular
  6. after meta.args on line 18 add:
decorators: [
  componentWrapperDecorator(story => `<div class="decorator">${story}</div>`)
],
  1. Add render+template to the Primary Story: (decorator only appears in source code snippet if there's a template)
render: (args) => ({
  template: `<storybook-button></storybook-button>`,
}),
  1. In the browser open Show code for the Primary button story and notice the decorator wrapping the button:
    <div class="decorator"><storybook-button></storybook-button></div>
  2. In the editor for button.stories.ts meta object, after the decorators config on line 21 add
parameters: {
  docs: {
    source: {
      excludeDecorators: true
    }
  }
}
  1. Return to the browser and notice that the decorator is now excluded
    <storybook-button></storybook-button>

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 77.4 MB 77.4 MB 12 B 1.89 0%
initSize 162 MB 162 MB 381 kB 3 0.2%
diffSize 84.7 MB 85.1 MB 381 kB 10.5 0.4%
buildSize 7.52 MB 7.52 MB 0 B 1.11 0%
buildSbAddonsSize 1.62 MB 1.62 MB 0 B 1.14 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 2.34 MB 2.34 MB 0 B 1.11 0%
buildSbPreviewSize 352 kB 352 kB 0 B - 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 4.51 MB 4.51 MB 0 B 1.11 0%
buildPreviewSize 3.01 MB 3.01 MB 0 B 1.11 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 7.5s 21.3s 13.7s 1.11 64.4%
generateTime 22.8s 22.3s -550ms 1.02 -2.5%
initTime 18.1s 17.4s -637ms 0.25 -3.6%
buildTime 11.4s 10.9s -526ms -0.74 -4.8%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 6.2s 7.7s 1.5s 0.66 19.9%
devManagerResponsive 4s 4.6s 567ms 0.1 12.2%
devManagerHeaderVisible 702ms 824ms 122ms 0.06 14.8%
devManagerIndexVisible 735ms 864ms 129ms 0.06 14.9%
devStoryVisibleUncached 1.1s 965ms -190ms -1.64 🔰-19.7%
devStoryVisible 736ms 862ms 126ms 0.04 14.6%
devAutodocsVisible 616ms 709ms 93ms -0.2 13.1%
devMDXVisible 667ms 713ms 46ms -0.13 6.5%
buildManagerHeaderVisible 734ms 728ms -6ms -0.2 -0.8%
buildManagerIndexVisible 744ms 734ms -10ms -0.44 -1.4%
buildStoryVisible 776ms 766ms -10ms -0.32 -1.3%
buildAutodocsVisible 630ms 763ms 133ms 0.76 17.4%
buildMDXVisible 639ms 636ms -3ms -0.5 -0.5%

Greptile Summary

This pull request addresses the Angular sourceDecorator issue by implementing the excludeDecorators flag functionality. Here's a concise summary of the changes:

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@shilman shilman changed the title Fix Angular sourceDecorator to apply excludeDecorators flag Angular: Fix sourceDecorator to apply excludeDecorators flag Sep 9, 2024
@valentinpalkovic valentinpalkovic self-requested a review September 10, 2024 08:28
Copy link

nx-cloud bot commented Sep 10, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 19a68df. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Copy link
Contributor

@valentinpalkovic valentinpalkovic left a comment

Choose a reason for hiding this comment

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

LGTM! @JSMike Thank you so much for your contribution.

@valentinpalkovic valentinpalkovic added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Sep 10, 2024
@JSMike
Copy link
Contributor Author

JSMike commented Sep 12, 2024

@valentinpalkovic let me know if you'd like me to rebase this to a single commit, or if you've got it from here.

@valentinpalkovic valentinpalkovic merged commit b0d55ed into storybookjs:next Sep 16, 2024
54 checks passed
storybook-bot pushed a commit that referenced this pull request Sep 16, 2024
Angular: Fix sourceDecorator to apply excludeDecorators flag
(cherry picked from commit b0d55ed)
storybook-bot pushed a commit that referenced this pull request Sep 16, 2024
Angular: Fix sourceDecorator to apply excludeDecorators flag
(cherry picked from commit b0d55ed)
storybook-bot pushed a commit that referenced this pull request Sep 16, 2024
Angular: Fix sourceDecorator to apply excludeDecorators flag
(cherry picked from commit b0d55ed)
storybook-bot pushed a commit that referenced this pull request Sep 16, 2024
Angular: Fix sourceDecorator to apply excludeDecorators flag
(cherry picked from commit b0d55ed)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Sep 16, 2024
@VincentPuget
Copy link

Thx !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
angular block: source bug ci:normal patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
4 participants