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

Transcluded clone is not compiled in 1.5.0-rc-0 in ui-select #13527

Closed
bogdanalexe90 opened this issue Dec 13, 2015 · 14 comments
Closed

Transcluded clone is not compiled in 1.5.0-rc-0 in ui-select #13527

bogdanalexe90 opened this issue Dec 13, 2015 · 14 comments

Comments

@bogdanalexe90
Copy link

After upgrading to 1.5.0-rc-0, ui-select has stopped working. After a short look it seems that they use a transcludeFn with a cloneAttachFn. The cloned element from cloneAttachFn is not compiled.

I created a plunkr to show the problem:
http://plnkr.co/edit/TB59p33RPPS0rgA01tlj?p=preview

@bogdanalexe90 bogdanalexe90 changed the title Transcluded clone is not compiled Transcluded clone is not compiled in 1.5.0-rc-0 Dec 13, 2015
@lgalfaso
Copy link
Contributor

It looks like ui-select is using an API that was deprecated in 1.2.1 90f8707

Unless someone comes with a small and clean patch, I think that this should not get fixed. Anyhow, will leave this open for other contributors to comment.

@bogdanalexe90
Copy link
Author

I think this should be marked as breaking change since many can have this issue

@lgalfaso
Copy link
Contributor

@bogdanalexe90 I agree that a notice is needed

@petebacondarwin WDYT?

@bogdanalexe90
Copy link
Author

And btw i tried to refactor and remove require but no change. Transclude is called before clone element is compiled & linked.

Just play a bit with some break points.

@petebacondarwin
Copy link
Contributor

@lgalfaso - Given that this was deprecated way back in 1.2.1 - do you think we could get away with removing it completely in 1.5? Especially if it seems that the transclude changes make it fail.

@petebacondarwin
Copy link
Contributor

Has anyone created an issue against ui-select for this too?

@lgalfaso
Copy link
Contributor

@petebacondarwin
Copy link
Contributor

Actually I am not convinced that ui-select is using a deprecated API. The API that was deprecated was to use the transcludeFn parameter on the compile function. The ui-select is using it from the link function, which is still supported.

I wonder if the actual bug is that our lazy compilation is not kicking in for the cloned transcluded content at the time it arrives in the clone attach function?

@lgalfaso
Copy link
Contributor

I think that I understand the issue. Things happen in this order:

Angular 1.4.x

  1. The directive ui-select is compiled
  2. ui-select wants to transclude the content
  3. ui-select-match starts compilation, it has a templateUrl so the compilation continues later
  4. ui-select-match template is resolved (as all the templates were already in the cache)
  5. ui-select-match compilation continues and modifies the DOM
  6. ui-select linking kicks in
  7. ui-select calls transclusionFn
  8. Inside ui-select transclusion handling function, there is a check that expects the modified DOM
    more steps that are not important

Angular 1.5.x

  1. The directive ui-select is compiled
  2. ui-select wants to transclude the content, lazy compilation kicks in
  3. ui-select linking kicks in
  4. ui-select calls transclusionFn
  5. ui-select-match starts compilation, it has a templateUrl so the compilation continues later
  6. Inside ui-select transclusion handling function, there is a check that expects the modified DOM. This triggers the error
    later and not important
  7. ui-select-match template is resolved (as all the templates were already in the cache)
  8. ui-select-match compilation continues and modifies the DOM
    more steps that are not important

It looks like ui-select is not waiting for ui-select-match to finish the compilation. Looks like ui-select has an inherit race condition that was mitigated due to the fact that the templates for ui-select-match are in the cache. This is, if the templates were in a slow server and ui-select would call transclusionFn before the server responded, then the same error would show up.

From all this, it looks like the issue is not the deprecated API

@lgalfaso
Copy link
Contributor

I wonder if the actual bugs is that our lazy compilation is not kicking in for the cloned transcluded content at the time it arrives in the clone attach function?

This was never warranted, only that the cloned element would be compiled and linked at some point in time (that may turn out to be when the server responds with the template or any other reason).

@LeleDev
Copy link

LeleDev commented Dec 13, 2015

+1... ui-select is the only library preventing me from update -.-

@Narretz
Copy link
Contributor

Narretz commented Dec 13, 2015

@lgalfaso is spot on, ui-select only worked 100% if the templates are put in the cache. See this plnkr for a contrived example: http://plnkr.co/edit/sRIW4LJ2VpSBJZAnODvY?p=preview

Sure, that's best practice and probably no user removed them from the cache. But that doesn't change the fact that ui-select should wait for the ui-select-match to be available. A parent directive reaching into its children without any safeguards ... seems wrong to me. ui-select-match / ui-select-choices should instead call on ui-select once they are linked. Similarly to how ngMessages does it.

@Narretz Narretz modified the milestones: Ice Box, 1.5.0-rc.1 Dec 15, 2015
@Narretz Narretz changed the title Transcluded clone is not compiled in 1.5.0-rc-0 Transcluded clone is not compiled in 1.5.0-rc-0 in ui-select Dec 19, 2015
@Narretz
Copy link
Contributor

Narretz commented Jan 27, 2016

Closing, since it's not an issue with angular core. ui-select also has a new contributor, so I'm confident this will be fixed soon.

@Narretz Narretz closed this as completed Jan 27, 2016
@jfairley
Copy link

looks like it was fixed angular-ui/ui-select#1430 by you 💯

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants