Options not applied within transcluded directive by VanTanev · Pull Request #123 · angular-ui/ui-ace

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.

VanTanev · 2015-09-12T14:41:24Z

I came across an issue where a wrapping directive within a transcluded directive would not be able to set ui-ace options. After stepping through the code, I found out the following:

When uiAce.link() is executing, the value of scope.$eval(attrs.uiAce) was undefined. However, when the scope.$watch(attrs.uiAce) was executed for the first time, the value was properly set to the expected object. However,

ui-ace/src/ui-ace.js

Line 279 in 4ce0309

if(current===previous)return;

short-circuits that first trigger of $watch, leaving us just the default options.

The proposed fix removes that short-circuit (which was not a performance issue in my testing). It also fixes the first call of updateOptions, which erroneously used options instead of opts.

By the way, in my testing, the directive wroks as expected without that updateOptions call when the short-circuit is removed. However, the tests fail. It seems like the tests are in the wrong here - they should probably wait a digest cycle before doing their assertions on the Ace Editor options.

In some circumstances, the value of `scope.$eval(attrs.uiAce)` will change between the execution of our link function and the first time `scope.$watch(attrs.uiAce)` is triggered. Since this is context-dependent, we cannot predict it. We could manually store "currentOptions" only for the purpose of short-circuiting that first call, but that would be a waste of effort, so just allow it to execute every time.