search

superhero-com / superhero-wallet

Superhero is a multi-blockchain wallet to manage crypto assets and navigate the web3 and DeFi space. Currently supporting Bitcoin, Ethereum and æternity blockchains.
https://wallet.superhero.com
ISC License
39 stars 38 forks source link

Avoid putting components into `data` object #852

Closed davidyuk closed 3 years ago

davidyuk commented 3 years ago

https://github.com/aeternity/superhero-wallet/pull/822/files#diff-04ce19239ddb42c4465e51f55ce85b5b24d0a635156e31077a0a8d74206cce57R55

From my perspective, it looks messy. There is are native API for object composition in Vue.

cc: @imollov

imollov commented 3 years ago

What about putting SVG as strings in a similar fashion?

Could you point me to the docs about Vue object composition you're talking about? I'd also appreciate it if you elaborate on your reasoning a bit further.

Thanks!

davidyuk commented 3 years ago

What about putting SVG as strings in a similar fashion?

In this case, you have to use v-html on the button side. It will cause the linter to warn us.

Could you point me to the docs about Vue object composition you're talking about?

I meant the ability to register a component and to use it in a template: https://vuejs.org/v2/guide/components-registration.html#Local-Registration https://vuejs.org/v2/guide/components.html#Reusing-Components

I'd also appreciate it if you elaborate on your reasoning a bit further.

Vue makes reactive all properties of data object. I'm not sure that there are not any edge cases when the component's object itself becomes reactive. In general, I would prefer to use data object only for the state of the current component (not its methods or sub-components).

Why don't write it the way how is was before?

      <BoxButton
         :text="$t('pages.account.tip')"
         :to="tippingSupported ? { name: 'tip' } : {}"
       >
         <Tip slot="icon" />
       </BoxButton>

I see that the way you did it is a bit shorter, but I'm not sure that breaking the default composition pattern worth it.

davidyuk commented 3 years ago

Today in the middleware frontend we have found that Vuex state and component state are precalculated on the server side, to pass them to the client they should be serializable. If we had a component with a render function in the state, we don't be able to send it to the client because functions can't be serialized.

Though, we have already several parts that put functions/components into the store's state, for example, the sdk instance and the way how modals module works. Would be nice to refactor them someday.