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

Addon-docs: Remove all defaultValue eval-ing #14900

Merged
merged 6 commits into from
May 18, 2021

Conversation

tmeasday
Copy link
Member

Telescoping on #14899

Issue: #14579

What I did

  • Simply do not eval and set argType.defaultValue in any case.

What I did not do

  • If argType.defaultValue is set, we continue to use it as the initial arg value (if the arg is otherwise unset).

    • This means we do not break backwards compatibility
    • Also it means we don't need to change the implementation of actions' argType enhancer
  • I think maybe we can clean up some test cases, @shilman we should probably walk through it together

How to test

  • Is this testable with Jest or Chromatic screenshots?

Yes, lots of stories will be affected.

@tmeasday tmeasday requested a review from shilman May 12, 2021 02:56
@nx-cloud
Copy link

nx-cloud bot commented May 12, 2021

Nx Cloud Report

CI ran the following commands for commit 9021343. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@tmeasday tmeasday force-pushed the 14579-remove-default-values branch from dd543f5 to 9021343 Compare May 14, 2021 05:21
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM!

@shilman shilman changed the title Remove all defaultValue eval-ing in addon-docs Addon-docs: Remove all defaultValue eval-ing May 18, 2021
@shilman shilman merged commit e78a021 into 14579-undefined-inputs May 18, 2021
@shilman shilman deleted the 14579-remove-default-values branch May 18, 2021 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants