-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
UI: Change edition to Octane #8418
Conversation
Without this, the data-test-* attributes used when invoking the component had nowhere to go.
Since template-only components are now Glimmer components, references that used to be available from this. now require the @ sigil.
There was a conflict between this being called attributes and the splattributes that are available for Glimmer components.
Ember Asset Size actionAs of 6fec3f2 Files that got Smaller 🎉:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! attributePairs
is as good a name as any.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This updates the Ember edition setting to Octane, which I removed from #8319
because it required the template-only Glimmer components setting to be turned
on, which broke some tests and seemed preferable to separate out.
It didn’t require that many changes, probably because there aren’t that many
template-only components. One template needed an
...attributes
to handlea test selector, a few templates needed
this.
changed to@
, and theAttributesTable
component needed itsattributes
property renamed toavoid collision with splattributes. I chose the somewhat-clunky
attributePairs
but I’m open to suggestions!
At first I was concerned that this would be a confusing developer experience,
not knowing whether to use
this.
or@
based on whether or not it was aGlimmer component, but
@
will almost always work, thankfully. One exceptionI came across while preparing this PR was
RescheduleEventRow
, whichuses an overridable
allocation
property to either accept an allocation beingpassed in or fetch one via
allocationId
, but that’s all I saw.