prettier in now integrated into our projects' linting. prettier is an opinionated (supports very minimal configuration) code formatter that essentially replaces all the formatting eslint rules, so in general eslint should be about code correctness (flagging references to undefined variables, enforcing super() calls in constructors, etc.) while prettier handles all the code formatting. Because it runs as an eslint plugin, it's well integrated into VSCode.
I personally have pretty mixed feelings about prettier. I think that code formatting is about communication, and I don't think that a one-size-fits-all-subject-matter approach is ideal for communicating. prettier definitely formats some things in a way that I think is less readable sometimes, but the broader Javascript community seems to be converging on it, and it's now included in Ember's default app/addon blueprints, so I think it makes sense to get on board. Maybe after using it for long enough, I'll change my mind about what is and isn't readable, but regardless, to the degree that the broader community adopts prettier, new developers and whoever else will have an easier time reading our code because if it follows the broad formatting conventions, so I think it makes sense for us to get on board.
It is possible to tell prettier to ignore certain files using a .prettierignore configuration file, or certain code blocks using a // prettier-ignore line comment. I haven't done this at all, and generally think it should be a last resort only after looking for ways to change the code to be more readable, e.g. if a line is wrapping in some hard-to-read ways, maybe that's a sign that we should reduce the amount of nested logic it contains by using more local variables or something. So while I don't think we should be ignoring files/code blocks very often, it is an option available to us.
There are also certain things that prettier pretty much ignores, such as multiple hbs blocks in rendering tests. In those cases when prettier autofixed the files, it often updated the indentation of the code surrounding those blocks, but didn't update the indentation of the content inside the backticks, resulting in odd formatting, e.g.
test('it does not track no-ops', async function (assert) {
this.sheet.undoStack.enable();
await render(hbs`
<Drawings::BatchHtmlEditor
@createOperation={{this.createOperation}}
@services={{this.services}}
@sheet={{this.sheet}}
@targetObject={{this.targetObject}}
@targetProp="value"
/>
`);
await editorPage.focus();
await editorPage.fillIn('<p>fire</p>');
await editorPage.fillIn('<p>the wheel</p>');
await editorPage.blur();
assert.equal(this.sheet.undoStack.canUndo, false);
});I did not take the time to go and fix all of those, so as you encounter them during development, feel free to fix them.
The commit that fixed up our code formatting according to prettier touched pretty much every file, so any code changes you have in a branch are virtually certain to have conflicts. Probably the easiest thing to do is to start over with a new branch off of main and manually re-apply your changes to the new branch. If you have a lot of code changes, it's probably going to be easier to do it via a rebase, but there are some tricks to making it easier, so hit me up and we can screen share/pair.
@ember/test-waiters now has a @waitFor decorator for telling the test framework to wait for entire functions, and it works for both async functions and coroutines, which are generator functions used to implement async behavior, such as in ember-concurrency tasks. This fully replaces ember-concurrency-test-waiter (where we would do @task({ withTestWaiter: true })) and mostly replaces our usage of waitForPromise(). Rather than wrapping each promise that we await or yield in a waitForPromise() call, we can just decorate the functions:
class Thing {
@waitFor
async doThing() {
await somethingAsync();
}
@task
@waitFor
*doThingTask() {
yield somethingAsync();
}There are still occasional cases where we need to wrap only a single promise, and in those cases it's still fine to use waitForPromise(), but @waitFor should be the default.
I've been growing increasingly dissatisfied with ember-cli-page-object -- it has a good deal of hidden magic, I often find it kinda unweildy to use because it doesn't just expose a DOM element for us to work with (we had to hack that in), it's annoying to use its non-native array API for collections, the distinction between a page object and a component definition thingy that can be embedded in another page object is super annoying, and it doesn't integrate that smoothly with qunit-dom or other testing utilities. So in my spare time I implemented a new page object library, fractal-page-object.
Please read through the README to familiarize yourself with it. The migration from ember-cli-page-object will probably be a slow and gradual one, but when implementing new page objects, please do so in fractal-page-object. Some conventions I'm starting to converge on:
- It's totally fine to implement a page object in the test file that uses it if only one test file uses it.
- In apps, let's put page objects that we implement in their own file in
tests/helpers/page-objects/ - In addons, let's put them in
addon-test-support/page-objects/
Ember has a pretty awesome new destroyables mechanism, which allows you to register destructors for objects and then tie their lifetimes ot the lifetimes of other objects. It's a pretty powerful mechanism that I think we'll find quite helpful over time, so please read over the RFC.
The story around controller/component lifecycles has changed in some ways that have significant implications for us. It comes in a few flavors.
Previously called render backtracking, mutate after render is when during a single render, we read a @tracked property and then later modify it. This is bad because it means the same property can have two different values during the same render, making apps less predictable/consistent.
The change that has some significant implications for us is that component constructors now run in the rendering context. So, if a component constructor reads and then modifies a @tracked property, it will cause problems. Similarly, if a component reads @tracked state in its constructor and then we use a modifier (like {{did-render}}) or helper to update that state, it will cause problems.
I don't think there was ever a guarantee of exactly when modifiers' hooks would be executed. In practice {{did-insert}} would run as soon as the element and its descendants were all rendered into the DOM and before any of its later sibling elements were, but this was not guaranteed. It looks like some changes to glimmer have made this no longer predictably the case. This means that our pattern, mostly in controllers, of using a {{did-insert}} (or other) modifier to initialize data on the controller that is also used for rendering is no longer reliable. So we have to stop relying on DOM order to support data initialization in controllers.
If a route is active when a test completes, the route's resetController() hook will not be called, which means the reset() hooks that ember-controller-lifecycle puts on our controllers won't run. So we can't rely on those hooks to do the kind of teardown that prevents memory leaks, or the tests will leak memory and slow down, and if the leak is big enough, cause the browser to run out of memory. So we have to rely on those hooks a lot less and instead do teardown in objects with shorter lifecycles like components or modifiers. I think it will take a little time to get comfortable with a new set of best practices, and hopefully ember-could-get-used-to-this (and future similar work that's planned) will help, once I/we have a chance to dig into it and, um, get used to it.