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

[Proposal] More Verbose Component Tree #331

Closed
wants to merge 1 commit into from

Conversation

heygambo
Copy link

@heygambo heygambo commented May 12, 2017

Someone on Stackoverflow was asking if it was possible to show individual component names in the component tree.

Subconsciously I always wanted this as well.
So why not give it a shot.

It works by implementing an optional computed value instanceName in the components.

plain_shell_und_vue-devtools_ working_copy__master 0_changed_files

This could be useful for lists but also for input components to show the name of the input.

Like:
<TextInput: first-name>
<TextInput: last-name>
<TextAreaInput: bio>
  • It has a note in the README.md
  • It has tests for the names
  • All previous tests still pass

Please let me know if there are any suggestions or critique.

Stackoverflow:
https://stackoverflow.com/questions/40936918/how-to-distinguish-between-component-instances-in-vue-devtools

Best,
Christian

This commit allows to have components show more verbose by letting components have individual names.
This can be useful for lists or forms.
@posva
Copy link
Member

posva commented May 12, 2017

I really don't like the need of creating a useless computed property with a specific name to make it work. However, displaying the key attribute may be helpful

@heygambo
Copy link
Author

heygambo commented May 12, 2017

Hey @posva,

thx for the comment.

You suggested using the key attribute. That's actually a pretty good idea!

I'll try to justify why I would still say something like the PR could be beneficial. Maybe we can change the implementation a bit.

There are 2 problems with this from my understanding.

Problems with having a key only implementation

There is a visual disconnect between what's shown on the browser vs what the component tree would show.

Demonstration with key only

Browser shows:
Choose your favorite Marvel character:
- Hulk
- Spiderman
- ...

Component Tree shows:
- <Hero: 1241>
- <Hero: 5235>
- ...

I would like it much better if I was able to have look at this:

Browser shows:
Choose your favorite Marvel character:
- Hulk
- Spiderman
- ...

Component Tree shows:
- <Hero: Hulk>
- <Hero: Spiderman>
- ...

key will most likely contain an id. I would find it better if I was able to set it to something that I can see in the browser so I can relate to the rendered result better.

We wouldn't be able to use this outside from loops.

It wouldn't be possible to use it for custom input components like this:

Form Component
<my-input name="first-name" v-model="firstName" />
<my-input name="last-name" v-model="lastName" />

Component Tree shows:
- <MyInput>
- <MyInput>

vs this:

Form Component
<my-input name="first-name" v-model="firstName" />
<my-input name="last-name" v-model="lastName" />

Component Tree with custom value
- <MyInput: first-name>
- <MyInput: last-name>

Having the key only solution wouldn't justify this feature in the first place in my opinion.

If the custom instance name version would make it into this PR, then we could have key be a fallback.

Computed Value Alternative

I think I might get why you would consider a custom computed value "useless". It mixes component business logic with vue tooling logic. If that's the reason then I totally get it and I'm against it too.

Here is an idea how to solve this. I'm not sure if that would be possible with vue-devtools changes only. Without changing anything in vue itself.

What if the component would look something like this:

export default {
  name: 'Hero',
  instanceName () {
    return this.name
  },
  computed: {
    // regular computed values.
  }
}

Would that be more approachable to you?

@posva
Copy link
Member

posva commented May 12, 2017

the real problem with key is actually the same that the computed property: we cannot force the developer to add a property to the component that only has an impact in devtools. If people start using the key attribute to simply have a more precise display on the devtools, that's misusing the key attribute.

I think I might get why you would consider a custom computed value "useless". It mixes component business logic with vue tooling logic. If that's the reason then I totally get it and I'm against it too.

About the other changes, there's a similar problem, we're adding code that has no impact in the app, it shall be removed when bundling or something else and it's not worth the cost... Even if it's call $debugName and we point out it's helpful when debugging, it shall also be removed from production code.

@heygambo
Copy link
Author

heygambo commented May 12, 2017

@posva

  1. Do you understand the some people's desire for this?
  2. Do you have any suggestions how to get there or close to that?

@ALL
Are there any other opinions?

@posva
Copy link
Member

posva commented May 12, 2017

yes, distinguish same components in dev tools to make it easier to select them
yes, the key (a name can be an id too), so it can help in some cases. Hovering will highlight the actual element in the dom too

@heygambo
Copy link
Author

Hovering will highlight the actual element in the dom too

that's true. okay I guess we could wait for other opinions or ideas how to make it work.

I'm not super familiar with the vue core so I might not come up with the best possible solution at this point.

Thx for your opinion again

@heygambo
Copy link
Author

Hey @armano2 thx for the thumbs down. Can you explain why you don't like this?

Best,
Christian

@heygambo
Copy link
Author

Here is an example how I use it atm.

screenshot 2017-05-22 15 42 27

@armano2
Copy link

armano2 commented May 22, 2017

Hello, right now we already have name property for component's I will prefer to see that name is reactive with possibility to pass function and let ppl do whatever they want there instead forcing us to make computed property with no side effect.

@heygambo
Copy link
Author

The computed property has been discussed already. We all agree it shouldn't go into the computed properties because that's where the business logic is and we don't want to mix it up with the tooling.

The latest suggestion is that it could be next to the name option.
I would argue that it is related to the component like the name option is as well.

export default {
  name: 'Hero',
  instanceName () {
    return this.name
  },
  computed: {
    // regular computed values.
  }
}

@heygambo
Copy link
Author

heygambo commented May 22, 2017

But it's becoming more obvious to me that nobody really cares about this. So maybe we close that and call it a day.

It was still interesting to me to make it work and I'll still use it in our project. Unfortunately I'm not sure how to make the version above work. So I'm going to keep the computed properties.

Still thx for the responses. I get your opinion.

@heygambo heygambo closed this May 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants