ember-polyfills / ember-angle-bracket-invocation-polyfill

MIT License
76 stars 33 forks source link

Does not work with (lazy-loaded) engines #47

Closed buschtoens closed 4 years ago

buschtoens commented 6 years ago

Only tested this with a lazy-loaded engine, but I would assume that eagerly bundled engines are also affected.

Since the runtime polyfill only patches the buildRegistry method of the Application class, it has no effect on engines:

https://github.com/rwjblue/ember-angle-bracket-invocation-polyfill/blob/53afc725e13d8db63bf8006c0c906969441482de/vendor/angle-bracket-invocation-polyfill/runtime-polyfill.js#L57-L60

Which is why it causes this error:

Uncaught Error: Assertion Failed: You cannot use 'SomeComponent' as a component name. Component names must contain a hyphen

I understand that Application extends Engine, but that this polyfill has to patch Application instead of Engine, because it needs to override some registry entries only defined in Application:

https://github.com/emberjs/ember.js/blob/v3.4.1/packages/%40ember/application/lib/application.js#L1091-L1093

I now tried running the same polyfill for Engine in addition to Application and so far it seems to work. 🤷‍♂️

This is not ideal for Application though, because the polyfill is:

Do you still think that this is a valid solution going forward? I will submit a PR then.

rwjblue commented 6 years ago

Definitely seems like a reasonable start, happy to help tweak it in a PR...

jbailey4 commented 5 years ago

My team recently ran into this as well when creating a new engine within our older application at ember@2.18.2. We wanted to install this as a way to start using some of the newer ember syntax, since the engine would be a greenfield project.

Happy to help with anything needed to get this fix in!

rwjblue commented 5 years ago

https://github.com/rwjblue/ember-angle-bracket-invocation-polyfill/pull/48 seems to have stalled a bit, any folks looking for this functionality willing to pick it up?

pgengler commented 4 years ago

I opened a new PR (#91) that's just the changes from #48 applied to current master. Tried it out in an Ember 3.1 app that uses engines and it seems to work.