feat($compile): optionally get controllers from ancestors only #4540
Conversation
|
Thanks for the PR!
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
|
CLA signed previously as Caitlin Potter |
|
Thanks Caitlin, I ran into this problem over the weekend. |
|
Any word on getting this merged in? |
|
I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let me know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
That's strange since she has other commits merged in to 1.2.4 |
|
I think that's an automated script doing that, @dtabuenc (I can't be sure, but since my inbox was full of those messages this morning, I have to assume). It's probably being worked on |
|
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
|
PTAL --- rebased since this got mentioned in #8511 again |
|
Has @IgorMinar or @tbosch given their blessing of the API change? |
|
It hasn't really come up, I almost forgot I had this patch open since it's been here so long |
|
why schedule this for rc2 now? I'd like a really convincing argument, especially a solid use case. otherwise we should move this back to backlog. |
|
Compelling use-case: recursive trees of directives (obviously there's a bit more involved, but this is one of the annoying things to hack around when it comes to component recursion). Alternative reasons: people think "^" means parent element, when really it means "the current element or any parent element". |
| @@ -1564,14 +1566,20 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { | |||
|
|
|||
|
|
|||
| function getControllers(directiveName, require, $element, elementControllers) { | |||
| var value, retrievalMethod = 'data', optional = false; | |||
| var value, retrievalMethod = 'data', optional = false, search = $element, prematch, match; | |||
|
My use case was nested directives, as in In my implementation of the directive I wanted the access the parent
controller, and not my own controller ^ ends up finding 'my own' controller, if you will, and not the parent controller. On Tue, Sep 9, 2014 at 10:19 AM, Igor Minar notifications@github.com
|
| ALL_OR_NOTHING_ATTRS = makeMap('ngSrc,ngSrcset,src,srcset'); | ||
| ALL_OR_NOTHING_ATTRS = makeMap('ngSrc,ngSrcset,src,srcset'), | ||
| REQUIRE_PREFIX_REGEXP = /(\?)|(\^\^?)/g, | ||
| REQUIRE_PREFIX_PRE_REGEXP = /^((\?)|(\^\^?))+/; |
There was a problem hiding this comment.
do we really need to regexps?
why not just /^((\^)(\^)?(\?)?)?/ and then check which groups matched?
There was a problem hiding this comment.
that will remove one of the regexps as well as the loop
There was a problem hiding this comment.
Doesn't that impose a requirement that ^ comes before ? if people want to do both? I thought about doing it that way but it just seems fragile usability-wise
|
suggested some change + we need docs update. otherwise lgtm |
| match = require.match(REQUIRE_PREFIX_REGEXP); | ||
| require = require.substring(match[0].length); | ||
|
|
||
| if (match[3]) { |
There was a problem hiding this comment.
I've made the requested changes, except added a way to ask for optional before specifying the search root. I think it's a lot more code this way actually.
petebacondarwin
force-pushed
the
master
branch
from
02dc2aa to
fd2d6c0
Compare
|
@caitp Could you add the changes to the docs as well? |
|
yes, one moment |
Implement option to strengthen require '^' operator, by adding another '^'. When a second '^' is used, the controller will only search parent nodes for the matching controller, and will throw or return null if not found, depending on whether or not the requirement is optional. Closes angular#4518
|
updated |
Implement option to strengthen require '^' operator, by adding another '^'.
When a second '^' is used, the controller will only search parent nodes for the matching controller, and will throw or return null if not found, depending on whether or not the requirement is optional.
It might be good to simply make this strengthened behaviour the default, since given what the original docs were saying, it really is the expected behaviour.
Closes #4518