Drop Lodash

[mrmlnc]

After talking with @voischev we think that the Lodash kills performance.

With Lodash

Without Lodash

[michael-ciniawsky]

👍 I investigate, if you have the time, even if it's not shaped up, maybe send somone send quick PR for that pls :). Otherwise I will do it, but need a few days before for new posthtml-expression release

[mrmlnc]

@michael-ciniawsky, Tomorrow will make PR.

[michael-ciniawsky]

:+1: Want to become one of the maintainers of this plugin then :)?

[michael-ciniawsky]

I will wait for PR, finish some pollishing then and finally make release 😛

[mrmlnc]

Well, I spent some experiments and until you use the cloning of Object/Array we don't talk about performance.

If I replace Copy/Merge on custom solution I can get maximum profit in 4-6 percent. If we want to get the most performance - need to completely reconsider the approach (without copy/merge AST and options).

[michael-ciniawsky]

If you can prototype a solution for that, I'm perfectly fine with bigger refactorings, atm finishing a few other posthtml related repos, then I will take a deeper look at posthtml-expressions :). Take your time meanwhile 😝

[mrmlnc]

Now posthtml-expressions addressed only for Node.js 6+, because uses spread operator, destructuring assignment and default values.

I thinks will be cool if this plugins can be work on Node.js 4+. We have three ways:

• Rewrite to least common features of ES6
• Use Buble (fast and very small) or Babel
• Rewrite to TypeScript 😄

P.S. I started to think of a new algorithm. Hopefully tomorrow will be the first result.

[jescalan]

Active LTS for node v4 ends in less than 5 months. It's probably time for people still using v4 to start the process of moving over to v6...

[mrmlnc]

Sounds reasonable for me. But I don't understand politics of PostHTML, because PostHTML supports Node.js 0.10+ and most plugins supports Node.js 4+ and 6+.

[jescalan]

I mean I don't really either. v0.10 is past end of life, so there's no reason to support it at all. If a plugin already supports v4, no reason specifically to remove that support, but it doesn't seem worth the effort to convert other plugins when v4 will exit active LTS so soon IMO.

[michael-ciniawsky]

But I don't understand politics of PostHTML, because PostHTML supports Node.js 0.10+ and most plugins supports Node.js 4+ and 6+.

Core is node v0.10+, reason maybe some legacy support for some the protein stuff, no idea. Plugins is node v4+ LTS (unofficially)

I thinks will be cool if this plugins can be work on Node.js 4+. We have three ways:

@mrmlnc Yep, node 4+ LTS should be the target imo. Although personally node v6+ LTS is perfectly fine, there are still many folks on node v4+ LTS for coporate reasons, personal 'lazyness', you name it :). It's also active LTS which should always be targeted. I don't like it, it's half-way to awesomeness in terms of language support, but can't be helped atm.

In terms of code style, must plugins use StandardJS. I'm not familar with typescript, but is it possible to config tslint.json to match StandardJS style, maybe something like StandardTS :D?. typings and ES6+ features, perfectly fine, go for it , but please at least no semicolons 🔥

For unit test the majority of plugins use AVA, some Mocha( 👎 😛 ).

But unified docs (README && JSDoc/?TSdoc?) are more important in terms of repo consistency imo (I can handle that, it's not popular dev work, but must be somehow done).

I started to think of a new algorithm. Hopefully tomorrow will be the first result.

👍 ask @voischev to add you to posthtml/collaborators team after PR (and :shipit: posthtml-mixins 💯 )

[mrmlnc]

Don't worry, for this plugin I will use your stack and code style 😎

[michael-ciniawsky]

Ok, like I said you can migrated the code base to typescript with at least a little StandardJS flavour, but then you are definitely 'commited' as a maintainer 😛 .

[voischev]

support nodejs v0.10 for this customer https://github.com/cloudflare

issue https://github.com/posthtml/posthtml/issues/105

ask @voischev to add you to posthtml/collaborators

np

[michael-ciniawsky]

support nodejs v0.10 for this customer https://github.com/cloudflare issue posthtml/posthtml#105

@voischev I see 😎 cool, yes in terms of QA and 'best practices', normally all plugins/repos should still support node v0.12+ LTS since its still active, even when in maintenance mode, but most plugins were authored when node v4+ LTS was likely to be the default. Nevertheless in the future (node v4 LTS +), we definitely must be more 'conservative' in terms of node version support to be taking seriously as a project 😛. Personal 'feature hunt' is not a good practice for a project, I'm also guilty here :)

np

👍

@mrmlnc posthtml-mixins must be here @posthtml then , very nice one so far, I take care of docs 💯

[voischev]

I believe (for plugins) that was the user and not asked for support, you can do as you like 😎.

but I vote for maximum possible support versions nodejs 🤓

stop offtop? 🤔

[michael-ciniawsky]

I believe (for plugins) that was the user and not asked for support, you can do as you like 😎.

Don't say what you maybe regret 😛

but I vote for maximum possible support versions nodejs 🤓

We are all 'ashamed' :) for our bad habits recently and follow Node Foundation's LTS Schedule from now on => Node v4+ LTS until 2018-04-01 😎

stop offtop? 🤔

Yep :)

[mrmlnc]

So,

After #27 this plugin was working twice as fast with keywords {{ key }}. But the good news ends.

Problem

In the current implementation, we have the following problems:

1. Deep copy and merge small/large objects
2. Deep copy AST

Benchmark: 7 op/s

Theoretical maximum

Consider the case when the loops are not expanding:

if (Array.isArray(target)) {
  for (let index = 0; index < target.length; index++) {
    m.push(node)
  }
} else {
  for (let key in target) {
    m.push(node)
  }
}

Benchmark: 368 op/s

Looking for problems

Just add Object.assign({}, data).

// the context in which expressions are evaluated
const ctx = vm.createContext(Object.assign({}, opts.locals))

Without deep copy we cannot implement loops, because we need to create a context for each iteration of loop.

Benchmark: 72 op/s

Conclusion

We cannot copy/merge objects or arrays inside the plugin, because this is kill performance.

Solution

Our only hope is to use a strings. Strings no have context, but they also cannot be used in the following plugins (running after posthtml-expression).

Well, i see two ways:

Strings

Try to use strings. For example, get the root node of loop and convert it to a string. Then replace all placeholders ({{ key }}) in the string and convert it back into an object.

Total number of actions: 1 (convert node to string) + n * items.length (copy and update string) + 1 (convert string to object)

Reshape idea

@jescalan came up with an interesting idea. Create an additional type of nodes - code, which are evaluated when generating HTML code. But I still do not fully understand (need to try later) what will be if I turn on after reshape/expressions, for example, plugin which handles strings.

<each loop="item in ['**bold**', '*italic*']">
  <p>{{ item }}</p>
</each>

Post conclusion

If you have any ideas - welcome 🌵

[mrmlnc]

Well, idea with string gave me hope.

Benchmark [5x loop with condition]

Currently not supported nested loops 😢

[michael-ciniawsky]

@mrmlnc 👍 Are nested loops doable with your concept?

[mrmlnc]

Closed by #32.