emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.45k stars 4.21k forks source link

Glimmer VM v0.22 broke hot reloading #15057

Closed toranb closed 7 years ago

toranb commented 7 years ago

I tested ember-cli-hot-loader today with ember v2.13.0-beta.1 and found that we are no longer able to clear the template/or compiler cache like we could previously with ember 2.12

A quick rundown of how the hot loader works => we clear all the cache we can find, then invoke rerender. Nothing magical but because ember 2.13.0-beta.1 takes a new version of the Glimmer 2 VM I'm wondering if you can help us identify how we should be clearing this cache now. I tried a few additional steps tonight, including what you see in the screenshot below (clearing not only _definitionCache, but also _compilerCache and _templateCache) but still no luck. I did step through the code to confirm it is executing as I found it was in ember 2.12 so my first guess is ... something has changed in glimmer 2 that we aren't yet clearing :)

screen shot 2017-03-22 at 8 56 44 pm

A full example app to show this broken can be found here. Any help would be amaz! Thank you in advance!

rwjblue commented 7 years ago

TBH, I don't really think this is an ember bug. The hot reloader is definitely digging around in some deeply private territory. I'm happy that folks enjoy using it, but at the moment it is definitely not using public APIs.

I think a good long term solution would be to truly analyze the need here, and submit an RFC to make the minimal required API that tools like this could hook into. That would give hot reloader (and other tools) the ability to stop using private API's...

toranb commented 7 years ago

@rwjblue understood - thanks for the quick reply!

rwjblue commented 7 years ago

To be clear, my reply wasn't trying to "shut you down" or anything. As usually I'm happy to help try to find a path forward with you...

toranb commented 7 years ago

@rwjblue not to worry - my plan is to pause for the moment (as we have a big event next week). I really have 2 choices worth considering

1) reboot and do the RFC you mention 2) play hero and reverse engineer the internals with each glimmer VM change

@MiguelMadero and I have had great success to this point and I feel on some level the RFC is giving up. No offense to your comment ^^above asking me to play by the rules but this has been a powerful tool (for me personally) and I'd prefer to keep it moving (instead of rebooting /asking for feedback/ getting everyone involved when truly that addon was built for me)

MiguelMadero commented 7 years ago

Hi,

tl;dr; let's extend this RFC or write a new one.

While I find the play hero approach fun, I also think that this could have a great wide impact if we could keep it stable for more than one Ember release. Additionally, even pre-glimmer 2, there are many known issues with the current hot-loader approach based on the component helper. Those issues are fixed with the AST approach, but we have not been able to release the new approach since it doesn't work with any Glimmer 2 version. I think it's about time to bite the bullet and do the less exciting work of writing that RFC.

I was talking with @chadhietala yesterday about this at the SF Ember Meetup and he said he could help me push this during the event next week and also recommended to look into an existing RFC that talks about the ComponentManager, I think we was referring to https://github.com/emberjs/rfcs/pull/213. I've not had a chance to review it, but that might be an "official" way to solve this.

Going forward, I think we need help from someone with Glimmer 2 experience to 1) "play hero" together one more time to get a working version for the AST approach and 2) write the RFC so we can keep this stable going forward.

MiguelMadero commented 7 years ago

Apologies, the following is pretty rough, just an initial brain-dump. I'll polish it a bit and send a comment to the RFC to move that discussion there.

I had a first pass through https://github.com/emberjs/rfcs/pull/213 and it seems promising. My initial thoughts are:

hot-replacement-component may be replaced by a hot-replacement-component-manager that looks something like:

class HotReplacementComponentManager extends ComponentManager<Object> {

  private components = new ComponentCache();

  create({ metadata }: ComponentDefinition, args: ComponentArguments) {
    let instance = getOwner(this).lookup(`component:${metadata.class}`);
    let otherValues = {path,otherValuesFromASTRewrite};
    return {instance, otherValues, metadata};
  }

  getContext({ instance }: Object): Ember.Component {
    return instance;
  }

  getView(createdItem: Object): Ember.Component {
    if (createdItem.otherValues.needsRefresh) {  // TODO: some how get this value from the service
      clearCache(createdItem.metadata.class); // Still need to clear require and container caches
      // magic happens here: 
      createdItem.instance = getOwner(this).lookup(`component:${createdItem.metadata.class}`);
    }
    return createdItem.instance;
  }

  update({ instance }: Object, args: ComponentArguments): void {
    instance.setProperties(args.named);
    instance.didUpdateAttrs();
    instance.didReceiveAttrs();
    // Plus other glimmer hooks, maybe simply call super here instead
  }
}

I'm making a lot of assumptions in our favor, but seems like the right direction.

toranb commented 7 years ago

@MiguelMadero first up - I wanted to apologize for speaking on your behalf yesterday. I respect your decision (leaning toward the RFC) because the total cost of ownership has grown out of control the past few months.

I'm conflicted because if someone has been using this addon with great success for some time and suddenly it's not supported /broken that reflects poorly on me (just something I'm dealing with here). I still plan to look deeper into the current break around mid April when I have the bandwidth but I certainly see value in moving ahead with the RFC as you suggested.

The truth (if people don't already know it?) is that you've done all the work anyway. My contribution was mostly in morale (to keep the dream alive/ help land something ember-cli might use itself one day). If you are headed one direction it's wise for me to consider the same as you are the brains behind that addon :)

MiguelMadero commented 7 years ago

I wanted to apologize for speaking on your behalf yesterday.

@toranb all good! I didn't feel like you were.

is that you've done all the work anyway

That's kind of you to say that, but the reality is that we just unlocked different parts of the puzzle.

.... it's not supported /broken that reflects poorly on me...

I agree that's why I think we need to do both in parallel, fix what's broken and start discussing those public APIs that will be required to keep this stable. Hopefully we can get some help from someone with more Glimmer 2 experience, since to me that has been the main blocker for both.