Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test {{in-element}} instead of deprecated {{-in-element}} #1067

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

mydea
Copy link
Contributor

@mydea mydea commented Jul 19, 2021

This leads to failing tests on Canary, where {{-in-element}} is removed.

This updates the test to test the {{in-element}} helper on ember versions upwards of 3.20. I did not include an alternate branch to test {{-in-element}} on older versions because I figured that is kind of repetitive and we can assume they work the same.

@mydea
Copy link
Contributor Author

mydea commented Jul 19, 2021

Canary tests will fail because of:
#1066

@mydea mydea force-pushed the fn/test-in-element branch from 027f628 to b6ceca7 Compare July 19, 2021 11:45
@simonihmig
Copy link
Contributor

simonihmig commented Jul 19, 2021

Canary tests will fail because of:

Actually 3.8 is failing. Probably because the error is thrown at build-time by the AST transform (which is not taking your if-clause into account), at least that's what I would assume. Maybe adding https://github.com/ember-polyfills/ember-in-element-polyfill as a devDependency instead of the Ember version check would help?

@mydea mydea force-pushed the fn/test-in-element branch from b6ceca7 to 5dfc343 Compare July 20, 2021 06:39
@mydea
Copy link
Contributor Author

mydea commented Jul 20, 2021

Hmm, interesting! I changed the test to just always run and use {{in-element}} and installed ember-in-element-polyfill as dev dependency. Should hopefully work now 😅

@rwjblue rwjblue merged commit 6a42636 into emberjs:master Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants