hybridsjs / hybrids

Extraordinary JavaScript UI framework with unique declarative and functional architecture
https://hybrids.js.org
MIT License
3.02k stars 85 forks source link

Attribute does not reflect to a property with descriptor #83

Closed nelsieborja closed 4 years ago

nelsieborja commented 4 years ago

Sample Code:

<simple-counter> Hybrids component:

{
  count: {
    connect: (host, key) => {
        console.log('count: ', host[key]);    // #1
        console.log('offset: ', host.offset); // #2
    }
  }
  offset: 1,
  render: ({ count, offset }) => html`
    { count: ${count}, offset: ${offset} }
  `
}

HTML markup:

<simple-counter count="2" offset="10"></simple-counter>

Behaviour and Output:

render method output: { count: , offset: 10 } console.log #1: count: undefined console.log #2: offset: 1

Expectation:

render method output: count has a value of 2 console.log #1: initial value of count is reflected from the count attribute console.log #2: picks up the value of offset attribute and overrides the default

I am a newbie and just playing around with the library before using it an App but then I came across the issue, perhaps there's something wrong with the code or might have missed something from the docs.

Btw, the simplified definition over class-based is a game-changer🤘🏽

smalluban commented 4 years ago

Hi @nelsieborja! Sorry for the late response (I was AFK lately). Ok, so about your problem... The library has some unique features. One of those is property translation. When you write offset: 1, under the hood it is translated to be offset: property(1), where property is a built-in property factory, which in this case returns a full object descriptor with the definition for number property with a default value of 1, which reads corresponding attribute value when it is connected.

If you pass an object as a property descriptor, the library only uses what you passed (read more in descriptors section). In your example, count is only defined with connect() method, so the property uses a default get and set (because of that you can read and write count value) but it has no default value or all features of the property factory.

The property factory accepts connect method as a second argument, so you can change your example to make it work:

import { property } from 'hybrids';

{
  count: property(0, (host, key) => {
      console.log('count: ', host[key]);    // #1
      console.log('offset: ', host.offset); // #2
  }),
  offset: 1,
  render: ({ count, offset }) => html`
    { count: ${count}, offset: ${offset} }
  `
}
nelsieborja commented 4 years ago

Thank you @smalluban for your response, that really explains when to use explicit property. Although in the property factory, host.offset still giving the value of 1 (default property) instead of 10 from the attribute - I am guessing getAttribute() is more appropriate here that is when it comes to dealing with default values? But then I am getting 10 after the hot reload.

Closing this in favour of property.

smalluban commented 4 years ago

It's not a good idea to relate to other properties in connect() method. The connect() is called sequentially each by each property in connectedCallback() of the HTMLElement. In your example, connect() method of the count is called before offset, so in that time offset is not yet set.

You should use get() method if your count property depends on offset. For example, you can define your count property like this:

const MyElement = {
  count: {
    get: ({ offset }) => offset + 1,
    connect: (host, key) => {
      console.log(host[key]); // It still logs "2" even you define attribute as "10"
    },
    observe: (host, value) => {
      console.log(value); // Logs "2" and then "11"
    },
  },
  offset: 1,
  // render factory uses "observe()" method to update the DOM
  render: ({ count }) => {
    console.log(count); // Logs "11"
    return html`<div>${count}</div>`;
  },
}
<my-element offset="10"></my-element>

The cache mechanism notifies other properties if dependencies have changed, and render is called with new values. As render uses requestAnimatinoFrame (it is called after connectedCallback() of all properties), in this example count in the first render call will be 11.

nelsieborja commented 4 years ago

@smalluban Thank you for sharing all these explanations really helpful and sorry for asking a lot of things. I am currently planning for a microfrontend architecture and I saw a huge possibility of doing it with Hybrids. Just understanding the behaviour of the attribute/property (the setting and updation of the values, relating properties, etc) bec I want to use Hybrids as the global store that can be shared across the application. Though I feel this will have the following drawbacks:

I took the global store idea from your example and my implementation attempt for this architecture is available here, in case you would ever want to take a look. Really appreciate your input reg using Hybrids as a global store. I have other few concerns like Routing, TypeScript and CSS-in-JS support but I believe such discussions already exist elsewhere.

smalluban commented 4 years ago

I am currently planning for a microfrontend architecture and I saw a huge possibility of doing it with Hybrids.

It's cool, that you want to choose hybrids in your project! as far as I know, the microfrontend architecture is about splitting parts of the project into separate independent modules written in chosen technology. It means that the state of the module is controlled inside, without the need to be global and co-operate between separate microfrontends. The glue is a backend APIs, or other data, which are then consumed by the parts. If in your case you still need a global store for the state I would suggest choosing more general technology, which then can be connected to all of your choices.

It might be a good idea to start with redux - it is super simple to use it with hybrids, and you don't need to create special "parent" element - just use connect() factory wherever you need some data from the store (Look at this example from the documentation - https://stackblitz.com/edit/hybrids-redux-counter?file=redux-counter.js). You can generate your actions outside of the modules, and invoke them in hybrids, as well as in React or other technology with sync. If you would go with one data object in AppStore concept it is almost the same as a redux concept (in how and when they update elements). The good idea is to connect particular data, not a "containers", for example not the whole state, but something deep like state.myData.cart.overalCount - then the cache mechanism will compare those values, not a reference to the current state, which might update often.

I have other few concerns like Routing, TypeScript and CSS-in-JS support but I believe such discussions already exist elsewhere.

Routing is not yet implemented in hybrids - for web components, I can recommend looking at the router from ionic team, or some from the webcomponents.org. However, the WC community did not yet create an answer for other reach solutions.

The TypeScript support landed in 4.1.0 yesterday 🎉. More info in the TypeScript section from the docs. I looked at your code example and I did some changes, so you can look at this here: https://codesandbox.io/s/romantic-greider-t8upw. I also fixed the problem with one of the properties, and change how your app-addcart communicate with the product element. You should avoid the situation, where your state is spread across multiple elements, like count in your example. Your app-addcart is an input controller, so you can make it as an input - where it's value changes, it can dispatch an "input" event. The product element can listen to this event and update own state.

💪Thanks to you I discovered two bugs with current support - the parent factory has wrong returning type (it's now highlighted) and dispatch helper function declaration is missing. I will fix those problems soon.

nelsieborja commented 4 years ago

I like Hybrids bec it's so "JavaScript" and so as your inputs and suggestions are brilliant. Can't thank you enough, really appreciate your time and for fixing the code, you're the best!

Reg store, I think Redux Toolkit would be the best choice, both globally (ie: App's common states) and within independent elements. But with Hybrids, I am going to stick with its built-in features.

The TypeScript support landed in 4.1.0 yesterday 🎉. More info in the TypeScript section from the docs.

Amazing! 🔥

💪Thanks to you I discovered two bugs with current support - the parent factory has wrong returning type (it's now highlighted) and dispatch helper function declaration is missing. I will fix those problems soon.

It's the least I can do.

Thanks again, cheers!

smalluban commented 4 years ago

@nelsieborja The store feature built in the library is live, so you might want to look at the docs and try it ;)