fix($parse): Preserve expensive checks when runnning $eval inside an expression#13850
fix($parse): Preserve expensive checks when runnning $eval inside an expression#13850lgalfaso wants to merge 1 commit into
Conversation
c900f6f to
b8a785b
Compare
|
Aren't |
b8a785b to
157e9e7
Compare
|
So, this works for I would still add tests for all of those (better safe than sorry 😃). E.g.: scope.foo = {w: $window};
expect(function() {
scope.$eval($parse('$$postDigest($eval("foo.w && true"))', null, true));
}).toThrow();
expect(function() {
scope.$eval($parse('$apply("foo.w && true")', null, true));
}).toThrow();
expect(function() {
scope.$eval($parse('$applyAsync("foo.w && true")', null, true));
scope.$digest();
}).toThrow();
expect(function() {
scope.$eval($parse('$watch("foo.w && true")', null, true));
scope.$digest();
}).toThrow();
expect(function() {
scope.$eval($parse('$watchCollection("foo.w && true", $eval)', null, true));
scope.$digest();
}).toThrow(); |
There was a problem hiding this comment.
Won't this break $$watchDelegate and inputs on fn?
There was a problem hiding this comment.
@jbedard you are right, this needs more work
|
How about disallowing execution of nested |
That would be too big of a breaking change |
|
People actually do that? I've never even thought of it. Unless there is a less obvious way that I haven't thought of... |
2add0ce to
39ecc9a
Compare
|
Updated the PR and fixed the comments. PTAL |
d529115 to
0e7bdaa
Compare
There was a problem hiding this comment.
Couldn't we move this inside the if-block ?
There was a problem hiding this comment.
Also, this would hide properties like assign, constant, inputs and literal.
There was a problem hiding this comment.
Also, wouldn't it make sense to cache the parsedExpression that has already had the expensive checks interceptor added, if the cache is cacheExpensive?
There was a problem hiding this comment.
Actually, that's what I meant by "move this inside the if-block". (Inside and before cache[cacheKey] = parsedExpression.)
Thx for making it clear, @petebacondarwin 😃
There was a problem hiding this comment.
Also, this would hide properties like assign, constant, inputs and literal
Given that the expensive checks is a private API, I think this is not an issue. Anyhow, made the change.
Couldn't we move this inside the if-block ?
Yep :)
997a7fd to
58e4014
Compare
|
LGTM |
…expression When running an expression with expensive checks, there is a call to `$eval` or `$evalAsync` then that expression is also evaluated using expensive checks
58e4014 to
5dbd8de
Compare
|
@petebacondarwin, should this (and #13871) be backported to |
|
@gkalpak - yes I would say so. Please do make it happen :-) |
…expression When running an expression with expensive checks, there is a call to `$eval` or `$evalAsync` then that expression is also evaluated using expensive checks Closes: #13850
|
Backported to |
When running an expression with expensive checks, there is a call to
$evalor$evalAsyncthen that expression is also evaluated using expensive checks