posthtml / posthtml-extend

Template extending (Jade-like)
MIT License
46 stars 9 forks source link

[feature] Add opportunities for use `extends` with the same attr's multiple times #47

Closed kudg0 closed 2 years ago

kudg0 commented 2 years ago

Hi!

I have the issue here: https://github.com/posthtml/posthtml-expressions/issues/126#issue-1125181767 But with help from thewebartisan7 i found out this cool plugin, thank you for your work!

When i go to use it, i saw that behavior of his work is't clear... If you use extends with the same attr's multiple times you have trouble with now fully replacing the nodes in parent block. Here is a problem: https://github.com/posthtml/posthtml-expressions/issues/126#issuecomment-1032293303

Another thank's thewebartisan7 for his help with understanding of issue and get the resolve, but in case where you have lots of template extends it's can be hard to implement(

Maybe we can add some improvements?)

Here is quick look through this problem and solutions: https://github.com/posthtml/posthtml-expressions/issues/126#issuecomment-1032529060

Quote from this comment:

Here is method from plugin code:

node_modules/posthtml-extend/lib/index.js

function getBlockNodes(tag, content = []) {
  const blockNodes = {};

  return (
    _api.match.call(content, { tag }, (node) => {
      if (!node.attrs || !node.attrs.name) throw getError(errors.BLOCK_NO_NAME);
      return (blockNodes[node.attrs.name] = node), node;
    }),
    blockNodes
  );
}

if we just add unique uid to returning nodes attrs, it's can be more useful for users

Here is that i mean:

function getBlockNodes(tag, content = []) {
  const blockNodes = {};

  return (
    _api.match.call(content, { tag }, (node) => {
      if (!node.attrs || !node.attrs.name) throw getError(errors.BLOCK_NO_NAME);
      return (blockNodes[node.attrs.name + Object.keys(blockNodes).length] = node), node;
    }),
    blockNodes
  );
}

Because our extendsBlockNodes and layoutBlockNodes always have the same number of uses, seems like is can be worthy resolves.

thewebartisan7 commented 2 years ago

Good catch! I have not time at the moment to check into but I think can be solved somehow you describe.

Maybe try run test (https://github.com/posthtml/posthtml-extend/blob/master/test/extend.js) and wrote your own for your change to see if all pass correctly, and in case try submit a PR.

I will also check into ASAP.

kudg0 commented 2 years ago

Yep! My hacky way to resolve this task is't good, cos it's change input data and that's not good for pass the tests))

I think good way here is to change method of collect data from objects...

I check it myself when i get more free time

I'll stay in touch, thanks!