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/Vue: Fix string docgen #14200

Merged
merged 2 commits into from
Mar 11, 2021
Merged

Addon-docs/Vue: Fix string docgen #14200

merged 2 commits into from
Mar 11, 2021

Conversation

tmeasday
Copy link
Member

Issue: #14056

What I did

Ensure we don't accidentally try and fix react-docgen-typescript when we are in vue

How to test

  1. There is a test for what the input looks like in vue3
  2. There is also a story in vue3, although it should be noted we don't current ever look at these stories :/

@github-actions
Copy link
Contributor

github-actions bot commented Mar 11, 2021

Fails
🚫

PR is not labeled with one of: ["cleanup","BREAKING CHANGE","feature request","bug","documentation","maintenance","dependencies","other"]

Generated by 🚫 dangerJS against c5b260a

@shilman shilman changed the title Fix issue introduced into vue docgen w/ strings Addon-docs/Vue: Fix string docgen Mar 11, 2021
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.

So the presence of func means we're in vue? Seems a little brittle, but the real solution is probably a major refactor so I can live with it.

addons/docs/src/lib/docgen/createPropDef.ts Outdated Show resolved Hide resolved
description: 'Hey! Hey!',
defaultValue: {
value: "'Default'",
func: false,
Copy link
Member

Choose a reason for hiding this comment

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

Is func always false in these cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose func is probably true when you are using one of them vue default value "generators". I haven't explored the full space of everything vue can do though :/

@tmeasday
Copy link
Member Author

tmeasday commented Mar 11, 2021

So the presence of func means we're in vue? Seems a little brittle, but the real solution is probably a major refactor so I can live with it.

Yeah, given the plan is to drop all this default value shenanigans in 6.3, my feeling is we just do what's easiest for now, especially given a refactor this close to 6.2 would be a bit scary.

@shilman shilman added the bug label Mar 11, 2021
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 merged commit f2125d8 into next Mar 11, 2021
@shilman shilman deleted the 14056-fix-vue-string-defaults branch March 11, 2021 13:33
This was referenced Mar 11, 2021
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