derbyjs / saddle Goto Github PK

It is impossible to enter negative numbers (i.e: -1) or float numbers (i.e: 3.5) in inputs with type="number".

Saddle throws an error when trying to append an each block containing nothing. This situation can occur for example when filtering out items with an if sentence from a list being displayed with an each block.

{{each list as #item}}
  {{if #item.show}}
    <p>{{#item}}</p>
  {{/if}}
{{/each}}

The error occurs when trying to access node.previousSibling on a node that is null. See https://github.com/derbyjs/saddle/blob/master/index.js#L819.

See https://github.com/dadisn/derby-bugreport for how to recreate the bug.

Screenshot of bug and error message can be found here: https://www.dropbox.com/s/s9e6mmz4tbb3avj/Screenshot%202015-01-12%2013.33.45.png?dl=0

expect.js is not actively maintained anymore. Chai is almost a drop-in replacement for expect.js. We switched to it in ShareDB without much trouble, so we should switch to Chai here as well.

See my comment in the following commit:

7395b7b#commitcomment-5657848

Current npm version is 0.0.2.
https://npmjs.org/search?q=saddle

For derby-templates 0.1.0 version is needed.
Please, publish new version to npm.

There is no code in DynamicHtml.update for recreating "start" and "end" comments with new binding.

So, finally it leads to an error:

Uncaught TypeError: Cannot read property 'nodeType' of null

Call stack is something like this:

createHtmlFragment index.js:388
DynamicHtml.update index.js:379
Binding.update

Test:

  it('updated raw HTML should have start/end comments', function() {
    var template = new saddle.Template([
      new saddle.DynamicHtml(new expressions.Expression('html'))
    ]);
    var binding = render(template, {html: '<b>Hi</b>'}).pop();
    expect(fixture.innerHTML).equal("<!--html--><b>Hi</b><!--/html-->");
    binding.update();
    expect(fixture.innerHTML).equal("<!--html--><b>Hi</b><!--/html-->");
  });

Rough patch is:

DynamicHtml.prototype.update = function(context, binding) {
  // Get start and end in advance, since binding is mutated in getFragment
  var start = binding.start;
  var end = binding.end;
  var value = getUnescapedValue(this.expression, context);
  var html = this.stringify(value);
  var fragment = createHtmlFragment(binding.start.parentNode, html);
  //++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  var startNew  = document.createComment(this.expression);
  var endNew    = document.createComment(this.ending);

  fragment.insertBefore (startNew, fragment.firstChild);
  fragment.appendChild  (endNew);
  updateRange(context, binding, this, startNew, endNew);
  //++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  replaceRange(context, start, end, fragment, binding);
};

Sorry, but 0.3.0 is too unstable. Binding errors, wrong dom-elements updates, if/else blocks doesn't switch. I downgrade to 0.2.0.

If ConditionalBlock is inside of EachBlock you use one last variable for all the instances of "if/else".
So. If you try something like:

index.coffee

app.get "/", (page, model) ->

  model.set('_page.items', new Array(10));
  model.set('_page.num', 0);

  page.render()

index.html

<Body:>
  {{each _page.items as #item, #index}}
    {{if #index == _page.num}} A {{else}} B {{/}}
  {{/}}

  <input type="number" value="{{_page.num}}">

And if you manually change input to 6 (for example) - you will get: AAAAAAABBB instead of BBBBBBABBB

I think you should keep last variable in the "if/else" context instead of in the ConditionalBlock instance.

Test:

  it('if/else inside each block', function() {
    var template = new saddle.Template([
      new saddle.EachBlock(new expressions.Expression('items'), [
        new saddle.ConditionalBlock([
          new expressions.Expression('flag'),
          new expressions.ElseExpression()
        ], [
          [new saddle.Text('A')],
          [new saddle.Text('B')]
        ])
      ])
    ]);

    var data = {items: [0,1], flag: true};

    var bindings = render(template, data);

    expect(getText(fixture)).equal('AA');

    var eachBinding = bindings[4];
    var if1Binding = bindings[2];
    var if2Binding = bindings[0];

    eachBinding.context.data.flag = false;
    if1Binding.update();
    if2Binding.update();

    expect(getText(fixture)).equal('BB');

  });

You should place it inside testBindingUpdates function (in the test.mocha.js)

As a workaround I just comment the line in code: //if (value === this.last) return;

Bug: Derby throws error on an input that should work.

The code to reproduce with:

app.get '/', (page, model) ->
  q = model.at 'col.id'
  q.subscribe (err) ->  
    model.ref('_page.msg', q.at('msg'))
    page.render()

<Body:>
  <textarea rows="10">{{_page.msg}}</textarea>

How to reproduce:

• Focus into the textarea
• Press enter twice
• Press F5 and watch the JavaScript Console

The troublesome function: https://github.com/derbyjs/saddle/blob/master/index.js#L1082

Derby runs its DOM-required tests using JSDOM now:
derbyjs/derby#569

We should get Saddle's tests on JSDOM too, and get them running on CI, probably GitHub Actions.