ManuelDeLeon / viewmodel

MVVM for Meteor
https://viewmodel.org
MIT License
205 stars 23 forks source link

'view' property added to template instances: need a warning or something #286

Closed steph643 closed 7 years ago

steph643 commented 7 years ago

The concept of view is common in programming.

For example, the ProseMirror text editor requires that you create an object of type EditorView in onRendered and that you destroy it in onDestroyed. So you naturally end up adding a view property to your template instance...

Well, if you do so while using ViewModel, you break your app in subtle ways. I've just spent a day on this. I guess the reason is that ViewModel has its own view property it adds to template instances.

Some ideas:

  1. Rename the ViewModel view property to something like __ViewModel__view

  2. Change the error message to something clearer. The one I got was :

    Exception from Tracker afterFlush function:
    meteor.js?hash=27829e9…:930 Error: Can't use $ on template instance with no DOM
    at Blaze.TemplateInstance.$ (blaze.js?hash=813922c…:3572)
    at viewmodel.coffee:217
    at Object.Tracker._runFlush (tracker.js?hash=9f8a0ce…:539)
    at onGlobalMessage (meteor.js?hash=27829e9…:401)
ManuelDeLeon commented 7 years ago

"view" isn't a keyword in ViewModel. How do you repro it?

On Mar 10, 2017 6:21 AM, "steph643" notifications@github.com wrote:

The concept of view is common in programming.

For example, the ProseMirror http://prosemirror.net/ text editor requires that you create an object of type EditorView in onRendered and that you destroy it in onDestroyed. So you naturally end up adding a view property to your template instance...

Well, if you do so while using ViewModel, you break your app in subtle ways. I've just spent a day on this. I guess the reason is that ViewModel has its own view property it adds to template instances.

Some ideas:

1.

Rename the ViewModel view property to something like ViewModelview 2.

Change the error message to something clearer. The one I got was :

Exception from Tracker afterFlush function: meteor.js?hash=27829e9…:930 Error: Can't use $ on template instance with no DOM at Blaze.TemplateInstance.$ (blaze.js?hash=813922c…:3572) at viewmodel.coffee:217 at Object.Tracker._runFlush (tracker.js?hash=9f8a0ce…:539) at onGlobalMessage (meteor.js?hash=27829e9…:401)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ManuelDeLeon/viewmodel/issues/286, or mute the thread https://github.com/notifications/unsubscribe-auth/AED31sm2sG8rXUvb134veBaCsqPG_JLtks5rkU5UgaJpZM4MZYJF .

steph643 commented 7 years ago

"view" isn't a keyword in ViewModel.

You are right, it is a keyword in Blaze! (see here)

In case it is useful to someone, here is the (client-side) code to reproduce:

<body>
  {{>myTemplate name=getName}}
</body>

<template name="myTemplate">
  <div {{ b "click: anyBinding"}}></div>
</template>
Name = new ReactiveVar('John');

Template.body.helpers({
  getName: function() { return Name.get(); }
});

Template.myTemplate.onRendered(function () {
  this.view = null; // This is what triggers the bug
  Name.set('Alice');
});

Template.myTemplate.viewmodel({
});

Running this code produces the following output:

Exception from Tracker afterFlush function:
meteor.js?hash=27829e9…:930 TypeError: Cannot read property '_domrange' of null
    at Blaze.TemplateInstance.$ (blaze.js?hash=813922c…:3571)
    at viewmodel.coffee:217
    at Object.Tracker._runFlush (tracker.js?hash=9f8a0ce…:539)
    at onGlobalMessage (meteor.js?hash=27829e9…:401)

Removing this.view = null; solves the issue.