Not glimmer 2 friendly

[toranb]

Currently if you spin this up and proxy a component you will get the error Template.create is not a function

More detail here https://github.com/emberjs/ember.js/issues/14228

We are using a CP for layout (something Robert calls out as likely a problem in the above issue). Also I'm uncertain if it's a problem just yet, but it seems the Ember.HTMLBars.compile could be an issue. I'm hoping we can modify this proxy to be both ember 2.8 and 2.9 friendly. I'll report back when I know more :)

[MiguelMadero]

I think the ast rewrite is a bit more reliable and it might just work in 2.9

[toranb]

I solved the original template error that blocked this in the early alpha/beta series. I assume this will land in the 2.9.0-beta.5 that ships next.

https://github.com/emberjs/ember.js/pull/14390/files

The issue now is that rerender doesn't seem to actually update the given template so I'll need to look at this further before I can identify the root cause. I'm out of bandwidth until Wed night next week but I do have this as my number 1 priority so we can continue to provide feedback during the glimmer 2 beta cycle

[MiguelMadero]

That's awesome. I'm not sure why rerender wouldn't work. I have some time tomorrow, but seems like you have a good handle on the 2.9 issue. I will try to explore the AST approach. I think that will remove the need to use the CP layout and might even fix the rerender.

[MiguelMadero]

@toranb I had a quick look into how glimmer is getting the template, while I think that using a CP is nice since it's useful for the first time to dynamically generate a template, it won't re-execute that code during a re-render since we only get the layout during lookup-component and we never update that. I think all the magic happens here:

function lookupComponentPair(componentLookup, owner, name, options) {
  let component = componentLookup.componentFor(name, owner, options);
  let layout = componentLookup.layoutFor(name, owner, options);

  let result = { layout, component };

  if (layout && !component) {
    result.component = owner._lookupFactory(P`component:-default`);
  }

  return result;
}

We would have to test this, but it seems like this is slightly different than what I remember seeing earlier. It's possible that the AST rewrite will work around this issue since the "layout" is always static, we will just re-render the yielded block.

[toranb]

@MiguelMadero great follow up and it makes total sense. I can now see why the CP for layout is a limitation vs the AST rewrite approach you've mentioned a handful of times now.

[MiguelMadero]

I was looking into something related at work and I found this in Glimmer 2, seems like one of the missing pieces of this puzzle:

https://github.com/emberjs/ember.js/blob/master/packages/ember-glimmer/lib/environment.js#L76-L86

this._definitionCache = new Cache(2000, ({ name, source, owner }) => {
      let { component: ComponentClass, layout } = lookupComponent(owner, name, { source });
      if (ComponentClass || layout) {
        return new CurlyComponentDefinition(name, ComponentClass, layout);
      }
    }, ({ name, source, owner }) => {
      let expandedName = source && owner._resolveLocalLookupName(name, source) || name;
      let ownerGuid = guidFor(owner);

      return ownerGuid + '|' + expandedName;
    });

I won't have time to look into this until late this week and I would like to wrap up the AST tests and land that PR, but I wanted to leave this here in case this helps.

[toranb]

To my surprise ... I loaded up 2.9.0 stable today and it seems to be working just beautifully

[josemarluedke]

@toranb Ember 2.9.0 did no ship with Glimmer 2. It is now expected and available in 2.10.0 (beta.1)

[toranb]

@josemarluedke huh any tweet, blog or gh issue I can read to learn about why it was delayed? This is the first I've heard of it

[MiguelMadero]

@toranb i can confirm that glimmer was delayed. Essentially 2.9 === 2.8.2. The release was branched from the lts since glimme 2 wasnt 100% ready. The release blog post contains more official details.

[MiguelMadero]

It's odd, I replied via email yesterday but I don't see any of my messages.... Anyway, the release blog post. In summary:

In the Ember 2.9 beta blog post, we announced our intention to include the new Glimmer 2 rendering engine in the 2.9 release. However, after careful consideration, we have decided to temporarily hold off on the integration.

This means that 2.9 will ship with the same rendering engine as 2.8 and does not include any new features, notable changes or deprecations.

This is good for hot-loader, because that means that we still support the "stable" for 6 more weeks and have time to make it work correctly for Glimmer 2 before the official release.

[toranb]

Awesome! More time to get this working as you mentioned :)

[MiguelMadero]

Huh?

Keep in mind that 2.9 is not glimmer two, it was reverted. Can you try 2.10-beta?

On Sat, Oct 22, 2016 at 11:45 PM Toran Billups notifications@github.com wrote:

Closed #18 https://github.com/toranb/ember-cli-hot-loader/issues/18.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/toranb/ember-cli-hot-loader/issues/18#event-832960263, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5HIa_froek8Ypgo7rfnET1cNhlUW6ks5q2soxgaJpZM4J5tLo .

[MiguelMadero]

Sorry for being the party pooper. We will make it work in Glimmer 2. I can have a look next week. On Sun, Oct 23, 2016 at 12:30 AM Miguel Madero me@miguelmadero.com wrote:

Huh?

Keep in mind that 2.9 is not glimmer two, it was reverted. Can you try 2.10-beta?

On Sat, Oct 22, 2016 at 11:45 PM Toran Billups notifications@github.com wrote:

Closed #18 https://github.com/toranb/ember-cli-hot-loader/issues/18.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/toranb/ember-cli-hot-loader/issues/18#event-832960263, or mute the thread https://github.com/notifications/unsubscribe-auth/AAC5HIa_froek8Ypgo7rfnET1cNhlUW6ks5q2soxgaJpZM4J5tLo .

[toranb]

@MiguelMadero no worries - I reopened the issue w/ the title "Not glimmer 2 friendly" (to be agnostic of ember version)

[GCorbel]

Is there any update on this issue ? Ember 2.10 will be released in two week and I'm facing to an issue that seems related to glimmer 2 and ember-cli-hot-loader. When I do a change on a component, it reload it (not the full page but just the component) but there is no change on it. It load the thing.

[MiguelMadero]

@GCorbel unfortunately not yet. As far as I know there is an issue in Glimmer 2 in the place where the cache is stored. I have not had a chance to look into it, but I'm working on a Glimmer 2 project, so I may be able to have a look at it tomorrow.

[toranb]

@GCorbel Sorry for my lack of involvement here. Robert Jackson did mention that a templateCache exists in ember-glimmer/environment

so … conceptually

let environment = owner.lookup(‘service:-glimmer-environment’);
environment._definitionCache /// bust this up

@MiguelMadero this sound like it's something we could pursue (while we wait on the bigger/better AST rewrite work you have planned) ?

[GCorbel]

@toranb you're right.

I tried with this instance initializers and it works :

export function initialize(appInstance) {
  appInstance.__container__.lookup('service:-glimmer-environment')._definitionCache.limit = 1
}

export default {
  name: 'glimmer-cache',
  initialize
};

There is probably a better way to this.

@MiguelMadero and @toranb don't worry about the lack of work on this issue. This an open source project and you do not have to work on it and, more importantly, you have not to share it for free. Thanks for your work!

[toranb]

@GCorbel wow that's an epic workaround honestly :)

no worries on the work here -we had a blocker originally and it seems like we have a way forward here so 2.10 will work when it's released in a few weeks. I'll knock out a PR that @MiguelMadero can look over in the next 2 days. Ideally you just do what you did in ember 2.9 and "magic" it's reloaded :) without any special initializer work.

I'll report back tomorrow once I get something up -thanks again for keeping this alive! Honestly didn't think people found this valuable (yet) as it's so young #good_to_know

[GCorbel]

I known this project by reading this article which was listed in the ember weekly of this week. It's very useful.

It seems I have this error message Error: EEXIST: file already exists, symlink '/frontend/tmp/broccoli_merge_trees-input_base_path-7gijyz6I.tmp/1/log-book/pods/components/delete-btn-form/template.js' -> '/frontend/tmp/broccoli_merge_trees-output_path-Mceiwyqw.tmp/log-book/pods/components/delete-btn-form/template.js' more often as issues because of the hot loader. It happens every 5 or 6 saves.

[MiguelMadero]

@GCorbel @toranb great finds. We can try to push this to our current cache invalidation hook and I can test in our 2.10 apps now that we have some of them :D

Unfortunately, the AST rewrite doesn't solve this problem, so this fix is still needed. The AST rewrite will simplify a few things and likely solve other issues, but I'm happy if we can get this one first.

@GCorbel re: EEXIST, can you open another issue and we can track it there?

Thanks for all your feedback and the workaround and I'm happy to hear that others are finding this useful, now we need to make it better :)

[MiguelMadero]

Woohoo! I had a quick look and using environment._definitionCache.store.clear() works, but we may be able to only remove the stuff we need to update.

[MiguelMadero]

@GCorbel @toranb I opened https://github.com/toranb/ember-cli-hot-loader/pull/28

[corrspt]

@MiguelMadero @toranb Just wanted to say that I also think this project is very cool. I tried it in a new project (as in empty) and it worked very well. When I tried with my production app (which is not so small) I had a few problems, but I do want to try again ;)

This is a really good project, thanks for the effort!

[MiguelMadero]

@corrspt , Thanks for the kind words this is encouraging!

If you more info of any of the issues you found, please open an issue. We're always looking for opportunities to make this better.

[corrspt]

When I get around to try it again (and I will, soon I hope), I'll create the issue. The problem at the time was that I couldn't "isolate" what was causing the issue so that I could provide a reproduction. But I'll do better next time!

Thanks again!