-
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: Update Ember/Ember CLI to 3.20 #9641
Conversation
Ember Test Audit comparison
|
d537afd
to
956af58
Compare
956af58
to
185401b
Compare
f3584f5
to
3ab5ea5
Compare
3ab5ea5
to
9743b4b
Compare
This includes reverting to Ember Data 3.12 and simple manual merging.
This includes a name change of `contains` to `includes`.
Without this, the following error was happening: Expected modifier head to be a string, got [32,1] Thanks to amk221 on the Ember Discord for mentioning this solution: https://discord.com/channels/480462759797063690/608346628163633192/712604682429661194
This fixes an error: “cannot create a new tag for (thing) after it has been destroyed” Thanks to the suggestion here: emberjs/ember.js#16541 (comment)
I also manually fixed the dependent keys for totalMemoryUnits including a spurious totalCPU.
This is just a naming clash, these aren’t the now-forbidden testing modules.
I guess autofix couldn’t handle them being nested…?
The linked issue is still open 😳
From the Ember Discord, pzuraq: If you don’t return a value, it’ll rerun the getter Which IMO is better, if a little more expensive https://discord.com/channels/480462759797063690/608346628163633192/751200673323810927 These don’t seem onerous to rerun, so I’ve removed them.
After reconstructing this update on the main branch, I was getting this failure when trying to run anything: _helperPluginUtils.declare is not a function Another mysterious Babel failure that required clearing the lockfile 😞
6a64d42
to
e7beabe
Compare
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.
This looks nice and straightforward and all the tests pass ✨
There are just a couple extra long lines that were probably introduced by tooling.
@@ -23,7 +23,7 @@ export default class AgentMonitor extends Component { | |||
isStreaming = true; | |||
logger = null; | |||
|
|||
@computed('level', 'client.id', 'server.id') | |||
@computed('client.id', 'level', 'server.{id,region}') |
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.
Does eslint expect dependent keys to be sorted now?
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.
This is an autofix situation where it added the missing server.region
dependent key and also brace expansion, when it rewrites it doesn’t care about existing ordering, I guess. I can change it to stay in the same order if preferable!
'activeTaskStates.@each.name', | ||
'activeTasks.@each.name', | ||
'activeTaskGroups.@each.name' | ||
'activeTaskGroups.@each.name', 'activeTaskStates.@each.name', 'activeTasks.@each.name', 'taskGroup.{name,tasks}' |
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.
Hmm, is there a prettier vs. eslint battle going on? This line exceeds our 100 character rule.
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.
eep… it seems as though Prettier didn’t run on this autofix change (and others, probably…) I’ll add another commit to fix these 😞 thanks for pointing it out
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.
Okay, I added 7d2c00e with line length fixes for a few files. I wish I knew why this happened.
I found 23 other files with Prettier autofixes, could be a separate PR at some point 🤔
import attr from 'ember-data/attr'; | ||
import { belongsTo, hasMany } from 'ember-data/relationships'; | ||
import Model from '@ember-data/model'; | ||
import { attr, belongsTo, hasMany } from '@ember-data/model'; |
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.
I'm a big fan of this packaging change.
Why were these missed in the original commits? 🤔
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 doesn’t include Ember Data, as we are still back on 3.12.
Most changes are deprecation updates, linting fixes, and dependencies. It can
be read commit-by-commit, though many of them are mechanical and skimmable.
For the new linting exclusions, I’ve added them to the Tech Debt list.
The decrease in test count is because linting is no longer included in
ember test
.There’s a new deprecation warning in the logs that can be fixed by updating Ember
Power Select but when I tried that it caused it to render incorrectly, so I decided to
ignore it for now and address it separately: