Ast

[MiguelMadero]

[WIP: do not merge]

I'm still missing the interesting part for AST. I have a bug with inverse, but the first 3 commits do some of the cleanup, initial documentation and serve as a proof of concept of the transform.

Blockers

• Test Test a component with a yielded block and positional params #22 (positonal params)
• Test Glimmer 2.
• Test Only wrap the components that we can hot reload #14 only wrap components we can hot-reload. We infer this at build time. That means we'll also "hot-reload" even if the component isn't currently rendered and we won't create additional wrappers for components that won't change.
• Should fix actions (test Problem to find actions in routes #27)

Should it block?

• Fix Ember 1.13

Non-blockers

• Test tagless components #24 (tagless components). Missing repro steps. (cc: @SirZach)
• Test helpers. @toranb with he previous solution, helpers needed a bit of extra work on the resolver, once we have the AST they should just work.

This approach fixes a few of the open issues like #22 and others that need more test. It doesn't FIX Glimmer 2 since the lookupComponent seems still to be cached (need to get more details, this might be another regression or maybe there is a new cache to invalidate in 2.9).

[MiguelMadero]

@toranb can you have a look at this one? I changed the description a lot to split between blocking/non-blocking.

I need to port the tests from #27, but I think it will just work. Helpers may just work, but I don't want to block on testing since they don't work today. We don't have repro steps for #14, but once we're happy with this and release it (likely as 0.2.0) we can ask SirZach to test on his app, it may just work.

I might be able to fix 1.13. I have a personal interest, because we have a large app on 1.13, but by the other hand there are more interesting things in the future and we could probably get back to 1.13 later...

[toranb]

@MiguelMadero just need a rock solid code review w/ someone to use it?

[MiguelMadero]

Yes. We started using it today. We can wait a few days and get some traffic
to make sure we dont break things (and I have time to wrap up the
outstanding tasks).
On Tue, Nov 22, 2016 at 7:48 PM Toran Billups [email protected]
wrote:

@MiguelMadero https://github.com/MiguelMadero just need a rock solid
code review w/ someone to use it?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#25 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC5HM8fLtK7k8_XY3B1I-adI4Jo9zgkks5rA5tZgaJpZM4KMLvf
.

[toranb]

Using this w/ ember 2.9.1 and ember-cli 2.7 seems to result in a full page refresh on component change. What version of ember/and ember-cli are you using that is solid?

[MiguelMadero]

I tried CLI 2.7.0 (used by the dummy app) and for Ember 2.7 and 2.10-beta. Details below.

Can you paste the output of ember s and the SHA you're using? That's the problem I had last early today when I started looking into it, I had all the components from the dummy-app hardcoded, but I removed that on the latest commit on this branch.

10:17 PM $ ember --version
ember-cli: 2.7.0
node: 4.4.3
os: darwin x64

11:24 PM $ bower ls ember 
bower check-new     Checking for new versions of the project dependencies...
ember-cli-hot-loader /Users/mamadero/code/mine/ember-cli-hot-loader
├─┬ ember#2.7.3 (latest is 2.10.0-beta.3)

As an aside, we really need to add some good end to end tests for the known scenarios.

[MiguelMadero]

Quick update. There's also an issue on 2.10.0-beta.3-beta+d668480b. This might be another breaking change, now with the parser, looks like node.path.original isn't a string anymore. We may need different htmlbars ast plugins for different versions, including 2.7-2.9 work fine, 1.13 and Glimmer 2 break. Not sure from 2.0-2.6, hope they overlap with 1.13 and 2.7 instead of having yet another version. I'll look later.

[toranb]

@MiguelMadero quick update myself - I plan to test this with ember 1.13/ 2.0 / 2.4 / 2.6 / 2.8 / 2.9 and 2.10 over the weekend. So far I'm uncertain if this is solid enough for a release -do you agree or has my initial experience been unique to me?

[MiguelMadero]

I agreed. Not ready yet. 2.10 and 1.13 are broken and the other ones haven't been thoroughly tested either. I was planning to spend some time fixing 2.10 and 1.13, but other things came up. I may have some time later this weekend. Also the 3 projects where we have it use 2.10, so they are still on the current release.

…

On Fri, Nov 25, 2016 at 3:39 PM Toran Billups ***@***.***> wrote: @MiguelMadero <https://github.com/MiguelMadero> quick update myself - I plan to test this with ember 1.13/ 2.0 / 2.4 / 2.6 / 2.8 / 2.9 and 2.10 over the weekend. So far I'm uncertain if this is solid enough for a release -do you agree or has my initial experience been unique to me? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#25 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC5HFhb7N9rM2bT5vhL_felvcHm4PGTks5rB1WsgaJpZM4KMLvf> .

[MiguelMadero]

@toranb @oligriffiths bad news. I thought that the clearRequireJS cache would fix this, but there seems to be a different type of cache hiding in there. It turns out that when we use the {{component helper as we do today when overriding the layout, we re-render all that fine. However, when we have something like:

{{#hot-replacement-component baseComponentName="inline-template-classic"}}{{inline-template-classic}}{{/hot-replacement-component}}

The inner template doesn't get re-evaluated.

[MiguelMadero]

If anyone wants to have a look at this or pair on this, I think we can make good progress with an extra brain.

[MiguelMadero]

Current status:

• default (2.7). It works, verified manually
• broken in 1.13 (Se https://travis-ci.org/MiguelMadero/ember-cli-hot-loader) due to a difference in the AST traversal API.
• ember-release, beta and canary. They pass CI, but fail manual verification due to a caching issue)
• other versions passed CI, but were not verified manually

[toranb]

@MiguelMadero I really owe you some time on this! I may have some time free tonight actually (after 5:30pm CST) or some night next week (Tue/Thur/Friday?) Ping me on slack so we can sync up and find a time to pair on this boss :)

[MiguelMadero]

@toranb some night next week works. I'll contact you directly

[toranb]

Closing old issues as I prepare for the v1.0 release later this month. If you get involved or have more bandwidth in the future to persue this just let me know