-
-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ember 2.13.0-beta.1 is broken (using the glimmer 2 VM v0.22) #48
Comments
Reminding myself this is still a big issue - broken in the current stable ember build 2.13.0 |
Quick update, @jcollins1991 and I were pairing on this. It seems like in 2.13 we already have a ComponentManager, that has a reference to the old After that, I had another quick look and was able to get it working with a small patch to ember's source. I would like to find the right extension point. We're pairing again on this next week to try to find the right hook. @jcollins1991 you can have a look at emberjs/ember.js#15057 for a few more details. Please check emberjs/ember.js#15057 (comment) for a rough idea of what a |
One more update, looks like the cc: @rwjblue @chancancode |
emberjs/ember.js#15254 is the first steps to exposing custom managers behind a flag. We will likely make the curly manager resolve in that PR (so both custom and "default" managers follow the same path). /cc @twokul |
@MiguelMadero @jcollins1991 @rwjblue you guys rawk :) |
@toranb @MiguelMadero I'm addressing comments from @rwjblue so we can hopefully land this today/tomorrow |
That's awesome @twokul I won't have time this weekend, but I would like to try this sometime next week before even if it's directly from your branch. |
@mamadero / @toranb / anyone else that groks how this works - Lets setup a meeting to discuss. I am not 100% certain that all of the use cases here will be addressed by custom component managers, so I want to make sure I understand the constraints in this addon and can better represent these concerns as we develop more and more glimmer infrastructure... |
@rwjblue sure. We can set something up next week (mon-wed), Im OOO starting thurs for 2 weeks. I can show you what we debugged yesterday which could give you a good idea of what is needed. However, it may be more productive if we get something working on top of @twokul's branch to make things more concrete. Btw, I have a different handle on GH (@MiguelMadero) |
@MiguelMadero @toranb @rwjblue I'd like to be a part of this discussion if possible |
Sure. Lets coordinate on slack.
…On Sat, May 20, 2017 at 3:06 PM Alex Navasardyan ***@***.***> wrote:
@MiguelMadero <https://github.com/miguelmadero> @toranb
<https://github.com/toranb> @rwjblue <https://github.com/rwjblue> I'd
like to be a part of this discussion if possible
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#48 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC5HB1OcwjCJcrTm5hMLPxJPPvi_EbTks5r72PWgaJpZM4MmEZM>
.
|
@rwjblue any chance we could chat again about landing a PR to invalidate the template cache in ember > 2.12? This is what we are doing today and it's in part what broke as of ember 2.13+ function clear (context, owner, name) {
var environment = owner.lookup('service:-glimmer-environment');
if (environment) { // Glimmer2
environment._definitionCache.store.clear();
}
if (owner.__container__) {
clearIfHasProperty(owner.__container__.cache, name);
clearIfHasProperty(owner.__container__.factoryCache, name);
clearIfHasProperty(owner.__registry__._resolveCache, name);
clearIfHasProperty(owner.__registry__._failCache, name);
clearIfHasProperty(owner.base.__container__.cache, name);
clearIfHasProperty(owner.base.__container__.factoryCache, name);
clearIfHasProperty(owner.base.__registry__._resolveCache, name);
clearIfHasProperty(owner.base.__registry__._failCache, name);
} else {
clearIfHasProperty(context.container.cache, name);
clearIfHasProperty(context.container.factoryCache, name);
clearIfHasProperty(context.container._registry._resolveCache, name);
clearIfHasProperty(context.container._registry._failCache, name);
// NOTE: the app's __container__ is the same as context.container. Not needed:
// clearIfHasProperty(window.Dummy.__container__.cache, name);
// clearIfHasProperty(window.Dummy.__container__.factoryCache, name);
// NOTE: the app's registry, is different than container._registry. We may need this
// clearIfHasProperty(window.Dummy.registry._resolveCache, name);
// clearIfHasProperty(window.Dummy.registry._failCache, name);
}
} |
I have been a bit busy with other projects, but I'm still looking forward to wrapping this up. |
@MiguelMadero do you have any detailed notes from our previous meeting w/ @rwjblue that detail out the path forward for breaking cache? |
Really looking fwd to trying this. |
@Samsinite I'm honestly not sure myself. If you are curious and want to hack I'd start by taking a look at the You should know that the core team would prefer a RFC on the subject (not a hero like you/ or me hacking to make this addon work). I'm fine with either myself - just didn't want to get your hopes up that someone from core would get involved to revive this project https://github.com/emberjs/ember.js/blob/master/packages/container/lib/container.js#L33 |
Happy to pair on it, either on the hero hack or bouncing ideas to start
that RFC.
…On Mon, Oct 16, 2017 at 5:04 PM Toran Billups ***@***.***> wrote:
@Samsinite <https://github.com/samsinite> I'm honestly not sure myself.
If you are curious and want to hack I'd start by taking a look at the
factoryManagerCache in the container
You should know that the core team would prefer a RFC on the subject (not
a hero like you/ or me hacking to make this addon work). I'm fine with
either myself - just didn't want to get your hopes up that someone from
core would get involved to revive this project
https://github.com/emberjs/ember.js/blob/master/packages/container/lib/container.js#L33
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#48 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC5HHMRXsS70MaWW6yrBsAwVE5S5Sslks5ss-8UgaJpZM4MmEZM>
.
|
Thanks for the feedback guys. Does anyone feel like this has been explored enough as an addon to start to write an RFC that they feel could be accepted into core? I think it would be fun to just hack against at the moment honestly. |
IMO, this has been explored enough, but my opinion shouldn't stop you from trying. It would certainly help you gain better understanding about what to write on that RFC if you later decide to go down that route. |
Unfortunately, at this point I don't have time to own either, but I'm happy to help someone carry this forward. |
I looked into this a bit today. I gave this a try and it seems to make the addon work again with a vanilla ember cli project using I admit I do not understand everything about the underlying reasons a change like this works, but I do know Without a broader discussion about the future of the Ember component manager interface, it will be hard to come to a great solution where this addon will be able to avoid making changes which rely on private APIs to continue working. Let me know if you want me to open a PR with this change to get things working again in the meantime, and then also if anyone wants to continue that broader discussion so we can come up with a good public API solution for addons like this. |
@alexhancock I’d be happy to pull this in if it solves the ember 2.17 issue. A further private api debate has been prevanet but I’d gladly fix this for all those people stuck atm |
For anyone lurking ;) @alexhancock legit solved this for ember 2.13+ I'll get some feedback on the PR #52 and see if we can't release something before the end of the week :) Thanks again Alex!!! |
I just published |
🎉 Thanks for integrating that change! |
I opened an issue w/ the ember project to see if they can help us nail down exactly how we should be clearing cache now in ember 2.13X
emberjs/ember.js#15057
The example app to show this broken can be found here for anyone interested.
I tried a few additional steps tonight (like clearing the
_compilerCache
and_templateCache
) but no such luck. I'm hoping the ember core team can shed some light on the issue so we can continue to support hot reloading in the ember ecosystem :)The text was updated successfully, but these errors were encountered: