search

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

MIT License
76 stars 33 forks source link

Add support for built-in components added in 3.10 #66

Closed lukemelia closed 5 years ago

lukemelia commented 5 years ago
rwjblue commented 5 years ago

Thank you for reporting! Yes, I totally agree. We can absolutely allow the new syntax and polyfill it back to curly invocation...

simonihmig commented 5 years ago

I started working on this, as I need this myself. Expect a PR soon! :)

rwjblue commented 5 years ago

Awesome, thank you!

rwjblue commented 5 years ago

Updated the issue description to include a checklist, only one thing left and we can release

simonihmig commented 5 years ago

@rwjblue any preference how you would handle warnings vs. assertions?

I would tend to this:

Regarding the handling of comments: I guess the most common use case would indeed be the example you have given of customizing template linter rules. So for that case, what does the linter actually see, the original template or the AST after our transformations? In the former case this would work fine I guess, and we don't have to issue a warning at all (for comments), right?

simonihmig commented 5 years ago

unsupported attribute: warning

I guess we should also treat /data-test-.+/ as supported when ember-test-selectors is present!? (due to the popularity of that addon)

rwjblue commented 5 years ago

I would tend to this:

unsupported attribute: warning

👍

handlebars comment: warning ... So for that case, what does the linter actually see, the original template or the AST after our transformations? In the former case this would work fine I guess, and we don't have to issue a warning at all (for comments), right?

Not sure here. Maybe its fine to just ignore? The only example I could come up with where it matters at all is the template linter config example I gave in the other PR, but the template linter looks directly at the template source files on disk (e.g. it doesn't run our AST transforms at all) so it would still be "fine".

I think we should probably just do nothing for handlebars comments...

modifier: assertion (i.e. breaks the build)

I agree that this has to be the default, but it feels kinda bad to me. Specifically, not being able to reliably do <LinkTo @route="foo" @model={{this.whatever}} {{on 'click' this.doSomething}}></LinkTo> is a shame, but since that support is only in 3.10+ anyways (3.4 through 3.10 all assert when passing a modifier into an angle component invocation) I don't see much choice here.

rwjblue commented 5 years ago

I believe this is now completed, can you confirm @simonihmig?

jamescdavis commented 5 years ago

Will this end up in a release soon? :wink: :wink:

simonihmig commented 5 years ago

I believe this is now completed

Yes, I think so! Craving for a release as well! 😁

rwjblue commented 5 years ago

Released in v2.0.0!

jamescdavis commented 5 years ago

@simonihmig I'm pretty sure I know the answer based on empirical testing and glancing at the code, but this does not support splattributes, right?