Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

refactor($compile): allow adding directive collectors and extractors #6781

Closed
wants to merge 1 commit into from

Conversation

lgalfaso
Copy link
Contributor

Refactor $compile so it is able to accept directive collectors and directive extractors. Add all the current directive collectors and directive extractors that handle the current operations

This patch is to start the conversation if the refactor into directive collectors and directive extractors should be done at 1.3 or if this should wait to 2.0

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#6781)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

function locateDirectives(name) {
var result = [];
forEach(environment.directiveLocators, function(locator) {
result = result.concat(locator(name) || []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this potentially creates a lot of garbage for the GC to clean up... the compiler isn't really something which is called too frequently most of the time, but this potentially hurts more than it helps

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, can be changed to

var directives = locator(name);
if (directives) result.push.apply(result, directives);

@lgalfaso lgalfaso added cla: yes and removed cla: no labels Mar 21, 2014
Refactor `$compile` so it is able to accept directive collectors and
directive extractors. Add all the current directive collectos and
directive extractors that handle the current operations
@Narretz
Copy link
Contributor

Narretz commented Apr 6, 2018

This is out of scope at this point in AngularJS development.

@Narretz Narretz closed this Apr 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants