ability to add logging info when binding to non-existent properties about binding HOT 16 CLOSED

related to #23

from binding.

maybe it would also be useful to log when dirty-checking is used in a browser with O.o support.

from binding.

this came up in the gitter again today: https://gitter.im/Aurelia/Discuss?at=55140f63c25916c13d35183d

from binding.

I tried to look into this but got stuck at adding logging to property-observation.js

I changed the code there to inject the logger (similar to how it's done in other places)

System.register(["aurelia-logging"], function (_export) {
   var LogManager, _logger, ...
    ...
   return {
    setters: [
      function(_aureliaLogging) {
        LogManager = _aureliaLogging;
      }
    ],
    ...

and updated jspm_packages\github\aurelia\[email protected]\.jspm.json to include the new dependency, but this didn't work - until I also mentioned the new dependency in my app's config.js

System.config({
  "map": {
     ...
    "github:aurelia/[email protected]": {
      "aurelia-logging": "github:aurelia/[email protected]",
       ...
    },
   ...

why is that? Am I missing anything, or it's really necessary to update the main config.js?

from binding.

Hey- thanks for looking a this, you're on the right track. What you're running into is aurelia/binding doesn't have a dependency on aurelia/logging yet. You'd need to `jspm install aurelia-logging' (which would add the entries in config.js). config.js is typically maintained by jspm, usually you don't need to hand edit it.

The logging code might go higher up, in the ObserverLocater class, and should be configurable. Reason is certain observer classes are used in certain scenarios, depending on the browser's support for O.o, etc.

from binding.

Thanks for the explanations! Didn't realize jspm install is the way to change config.js

I'm quite new to all this so will need some more guidance. Can you look at my commit https://github.com/andreister/binding/commit/f616ec88494f22bf82e9a6b42b1e7fffc6a26f28 and see if it's close?

I'm not sure about your "...should be configurable...", what kind of configuration do you mean? I see configurable: true all over the code, but cannot yet understand how it's used.

from binding.

Adding property existence checks in AccessScope, AccessMember, CallScope, CallMember could be bad news for performance, especially with AccessScope's ability to find props on the ancestor scopes.

from binding.

Is there some way that we could have a debug mode...where when it is turned on the AST parsing substitutes different debug mode AST instances perhaps? Just thinking out loud here...

from binding.

Sounds like a good idea to me. Will need to make sure the test coverage is beefed up in those areas. I think we could modify some of the existing AST tests to enable reuse between AccessScope and "DebugAccessScope" if that's the way we go about implementing this feature.

from binding.

Sounds good. It would be nice to have something like this. We'll have to consider how it should be configured for debug mode. We may need to put a setting on the BindingEngine and then somehow surface that through the Aurelia FrameworkConfiguration object.

from binding.

The configuration piece will be important to nail down for #205 as well

from binding.

Good point.

from binding.

@jdanyow @EisenbergEffect Can we simplify it down to this line

When we try to do everything in getObserver but couldn't figure out the right observer, and the property doesn't exist in prototype chain, we log a warning ?

// Instead of
// return new SetterObserver(this.taskQueue, obj, propertyName);

// We do
this.logger
      .warn(`Attempted to observe non existing property [${propertyName}] on instance of [${obj.constructor.name}]. Fallback to SetterObserver`);
    return new SetterObserver(this.taskQueue, obj, propertyName);

from binding.

My only concern would be whether or not this causes a perf regression.

from binding.

we can add PLATFORM.isProduction property to use for this purpose, followed by changes in bundlers what do you think ? @EisenbergEffect

from binding.

@EisenbergEffect mentioned this one in vNext cases
this one can be closed I guess

from binding.