[bug3023] Expressions refactoring #32
Conversation
kachurun
added
enhancement
kachurun
force-pushed
the
bug3023
branch
2 times, most recently
from
00c4986 to
583a1a4
Compare
|
Thank you @kachurun for this PR but this code will reduce massively the riot performance since the value dirty checks were partially removed. I am working on the same issue and I might have found a solution to it so for now let's close this PR. |
| expression.node, | ||
| expression, | ||
| value, | ||
| expression.value, |
There was a problem hiding this comment.
I don't see any reason to pass 4 arguments when one of them already contains everything you need
| // hopefully a pure function | ||
| this.value = this.evaluate(scope) | ||
|
|
||
| // IO() DOM updates | ||
| apply(this, this.value) | ||
|
|
||
| return this |
There was a problem hiding this comment.
Reduce code duplication
| // ref attributes should be called only once during mount | ||
| if (this.name === REF_ATTRIBUTE && !this.value) { | ||
| value(this.node) | ||
| this.value = value | ||
| return this | ||
| } |
There was a problem hiding this comment.
I moved it here so that the mount/unmount logic is in one place. Removed any ref processing from attribute expressions.
But in any case it looks ugly here, I would prefer a dedicated expression type for ref.
| // otherwise we can try to render it | ||
| return typeof value === 'undefined' || value === null | ||
| function handleSpreadAttributes(node, value, oldValue) { | ||
| const newKeys = Object.keys(value || []) |
There was a problem hiding this comment.
Removed the double calculation of the object keys
| @@ -1,10 +1,8 @@ | |||
| import { isNil } from '@riotjs/util/checks' | |||
There was a problem hiding this comment.
This thing is not used anywhere in riot except this file, and it's too complicated because there is no difference between v=== null|| v=== undefined and just `v== null'
No description provided.