search

posthtml / posthtml-expressions

Use variables, JS-like expressions, and even markup-powered logic in your HTML.
Other
123 stars 20 forks source link

Incorrect work with loops and conditions #21

Closed mrmlnc closed 7 years ago

mrmlnc commented 7 years ago

Detail

Moved from https://github.com/posthtml/sugarml/issues/7.

I'm use sugar@0.1.0 (0.4.0 related to Reshape as I understand it) and I have following code:

index.js

'use strict'

const { readFileSync } = require('fs');

const posthtml = require('posthtml');
const sugarmlOld = require('sugarml-old');
const posthtmlExp = require('posthtml-exp');

const smlFile = readFileSync('./index.sml', 'utf8').toString();

posthtml([posthtmlExp({ locals: { name: 'world' } })])
    .process(smlFile, { parser: sugarmlOld })
    .then((result) => {
        res = result.html;
        console.log(res);
    });

index.sml

head
  title testing!
body
  p hello {{ name }}!
  ul
    each(loop='number in [1,2,3,4,5,6,7,8,9]')
      if(condition='number % 2 === 0')
        li {{ number }} % 2 === 0
      else
        li {{ number }}

After processing I get:

<head>
  <title>testing!</title>
</head>
<body>
  <p>hello world!</p>
  <ul>
    <li>{{ number }}</li>
    <li>2 % 2 === 0</li>
    <li>{{ number }}</li>
    <li>4 % 2 === 0</li>
    <li>{{ number }}</li>
    <li>6 % 2 === 0</li>
    <li>{{ number }}</li>
    <li>8 % 2 === 0</li>
    <li>{{ number }}</li>
  </ul>
</body>

Output without posthtml-exp:

<head>
  <title>testing!</title>
</head>
<body>
  <p>hello {{ name }}!</p>
  <ul>
    <each loop="number in [1,2,3,4,5,6,7,8,9]">
      <if condition="number % 2 === 0">
        <li>{{ number }} % 2 === 0</li>
      </if>
      <else>
        <li>{{ number }}</li>
      </else>
    </each>
  </ul>
</body>

Environment

mrmlnc commented 7 years ago

This is can be solved by: 

while (conditionals.slice(-2).indexOf(nextTag.tag) > -1) {
  // ...

  obj.content = walk(opts, obj.content) // <------------------ ⚽️

  ast.push(obj)
  ;[current, nextTag] = getNextTag(nodes, ++current)
}

But this is generate unnecessary whitespaces 😫

Expected:

<ul>\n\n\n      <li>1</li>\n\n\n      <li>2 % 2 === 0</li>\n\n\n
      <li>3</li>\n\n</ul>

Actual:

<ul>\n  \n    \n      <li>1</li>\n    \n    \n      <li>2 % 2 === 0</li>\n    \n
\n      <li>3</li>\n    \n</ul>

But this is behaviour was and before adding this line:

<ul>\n  \n    \n      <li>{{ number }}</li>\n    \n    \n      <li>2 % 2 === 0</li>\n    \n    \n      <li>{{ number }}</li>\n    \n</ul>
michael-ciniawsky commented 7 years ago

I haven't found time yet to take a look into it :), can you trim() within obj.content somewhere?

reshape-expressions #8 reshape-expressions #8 Fix

mrmlnc commented 7 years ago

@michael-ciniawsky, no pressure 👍 I was just interested to solve this problem to run my Benchmark. I'll try to send PR but later.

michael-ciniawsky commented 7 years ago

@mrmlnc What are you benchmarking 👀 , is it compared to or in general :)? If you can fix it I rename and publish asap 😛

mrmlnc commented 7 years ago

@michael-ciniawsky, I am looking for a replacement for Jade/Pug (veeeery slow for big pages: 2 loops, 2 conditions & 5 mixin references ~~ 1-2s/page).

I build simple stack with:

And...

🐌 results ![code_2016-11-10_11-14-25](https://cloud.githubusercontent.com/assets/7034281/20169978/68600aee-a73b-11e6-835e-ecc5b2d391fa.png) ![conemu64_2016-11-10_11-13-05](https://cloud.githubusercontent.com/assets/7034281/20169954/441fb27e-a73b-11e6-87c4-f71e4552f76c.png)
voischev commented 7 years ago

https://github.com/posthtml/posthtml-expressions/blob/master/lib/index.js#L3

michael-ciniawsky commented 7 years ago

I see, I have similiar intensions my 'dream' dev stack will/should look something like that

|- src
|    |-components
|    |-index.sml
|    |-posthtml.config.js
|- index.sss
|    |-components
|    |-index.sss
|    |-postcss.config.js
|- ...
|- package.json

postcss.config.js (done)

modules.exports = (ctx) => {
   return {
     parser: ctx.ext === '.sss' ? 'sugarss' : false,
     plugins: {
        'postcss-import': {},
        'postcss-modules': {},
        ...,
        'cssnano': ctx.env === 'production' ? {} : false
     }
   }
}

posthtml.config.js (needs update, in progress)

modules.exports = (ctx) => {
   return {
     parser: ctx.ext === '.sml' ? 'sugarml' : false,
     plugins: {
        'posthtml-include': {},
        'posthtml-expressions': { locals: ctx.locals },
        'posthtml-content': ctx.content,
        'posthtml-extend': ctx.extend,
        'posthtml-css-modules': {},
         ...(mixins || snippets)
         ...components // Plugin to write Single File Components with PostHTML instead of Vue.js
        'htmlnano': ctx.env === 'production' ? {} : false
     }
   }
}

component.sml (prototype)

style(postcss [critical])
   // Component CSS

template(id='[hash]', name='')
   // Component HTML

script(babel [critical])
   // Component JS

index.sml

doctype
  head
    style // Inline Single File Component Critical CSS here
    link(href='index.sss') // With non-critical Single File Component JS + Deps (@import)
  body
    include(src='include.html')
    component(src='component.html')

    script // Inline Single File Component Critical JS here
    script(src='index.js') // With non-critical Single File Component JS + Deps (import, require())

Besides that.. :), a mixin plugin is something important missing I tried a long time ago, but was not working as I wanted, interested in collab to find a good solution ?

michael-ciniawsky commented 7 years ago

@voischev What is the Issue with _.clonedeep ?

mrmlnc commented 7 years ago

@michael-ciniawsky,

interested in collab to find a good solution ?

firstly I'll try to update posthtml-sugarml, because right now it slows down the whole process. Then I think to rewrite benchmark for more realistic cases (include, extends and expressions) and compare performance. Right now Slm super fast but not have Mixins and plugins (not really important to me).

Now I not have a free time, but i put a note in Trello.

michael-ciniawsky commented 7 years ago

firstly I'll try to update posthtml-sugarml, because right now it slows down the whole process

You mean posthtml-expressions instead of posthtml-sugarml? Or maybe I'm not getting the context here :). If you meant posthtml-sugarml can you file issue(s) over there, containing want you have in mind, I'm also currently shaping it a little, but suggestions on the how and contributions are welcome anytime and highly appreciated 😛 .

Mixins and plugins (not really important to me).

posthtml-mixins let's draft/prototype/write that somehow 👍 .

mrmlnc commented 7 years ago

@michael-ciniawsky, Yeap, I mean posthtml-sugarml, because I think it's because of him posthtml gives such a bad result (I hope so).

And yeap, I can send PR tomorrow to posthtml-expressions (100%) and posthtml-sugarml (60%, because I have 57 assigned to me Issues on GitHub, pending fix 😄).

michael-ciniawsky commented 7 years ago

@mrmlnc a few minutes ago, there was an awesome PR out of the blue, which added scopes and a <scope> tag to posthtml-expressions, take your time of course no real urgency, but maybe, you can give it a portion of higher priority 👍 😛. Is there a fork of posthtml-sugarml with your pushed changes so far, so I can sneak peek into it ? ;) It's because I refactored some code already, but mainly cosmetics and simplifcation.

mrmlnc commented 7 years ago

@michael-ciniawsky,

Is there a fork of posthtml-sugarml with your pushed changes so far, so I can sneak peek into it

Nope, I just wanted to synchronize the current repository with Reshape repository.

jescalan commented 7 years ago

As mentioned earlier, I patched this exact issue in reshape/expressions a while ago, happy to patch it here as well if you guys want, it would be quick and simple to do 😁

mrmlnc commented 7 years ago

@jescalan, If you can do it, I would be happy ❤️.

jescalan commented 7 years ago

Done, with a test to prevent regressions. Sorry this bug existed in the first place, it was definitely my oversight!

mrmlnc commented 7 years ago

@jescalan, Thanks you ⭐️

voischev commented 7 years ago

@michael-ciniawsky cloneDeep very slow :(

michael-ciniawsky commented 7 years ago

@voischev 👍 yep, never used lodash and friends myself, off topic here, but I have a personal aversion against this, because I can require('crap') development model in general ;) although I'm NodeJS Dev