DEV Community: Ben Demboski

Upgrading to Ember 6.1: linting with type information

Ben Demboski — Sat, 01 Feb 2025 04:25:49 +0000

I recently upgraded all the packages in our very large monorepo to the Ember 6.1 app blueprint. The pretty modest file-by-file diff that appears to just upgrade some dependencies and switch to the new eslint flat config file format belies some very exciting and impactful changes to the code validations that eslint performs.

My pull request that upgrades to Ember 6.1 and updates all of our code to comply with the new default linting configuration included 13 commits containing changes to 12,100 lines of code across 1,155 files! This was virtually all driven by the change from @typescript-eslint's recommended configuration to its recommendedTypechecked configuration.

I learned a lot in the process of updating all of our code to comply with the new linting configuration, and I wrote a fairly long and detailed PR description to share what I learned with my team, so I figured I'd share it with the broader community in case it's helpful for others. And here it is!

Ember 6.1 includes some big linting updates, described below. So this PR updates us to Ember 6.1, including some miscellaneous/minor blueprint updates, but the bulk of it is making our codebase conform to the new linting rules.

New linting setup

One very notable change in the linting setup is that for ts and gts files, we've started using linting with type information, which allows us to use linting rules that are much more powerful and helpful. The major implications of this are described below.

@typescript-eslint/no-unused-vars

This rule was updated to also flag unused variables in catch statements, so where we used to do

try {
  doSomething();
} catch (e) {
  // ignore
}

we can now omit the variable declaration entirely and use a syntax I didn't even realize was allowed:

try {
  doSomething();
} catch {
  // ignore
}

@typescript-eslint/unbound-method

This rule helps us make sure we don't call methods with this bound incorrectly, e.g.

class Thingy {
  private readonly obj = { key: 'value' };

  setObjValue(val: string) {
    this.obj.key = val;
  }
}

let thingy = new Thingy();
let setObjValue = thingy.setObjValue;
// whoops, `this` will be `null`, so `this.obj.key`
// with throw an error
setObjValue('hello');

When I was fixing up our code to comply with this rule, I encountered some "false positives" that I addressed in a few different ways, and are informative for writing code in the future to comply with this rule.

The false positives mainly have to do with the fuzzy distinction between a class method and a property that happens to be a function. Consider:

class Thingy {
  classMethod() {}
  readonly propertyThatIsAFunction = () => {};
}
const thingy = new Thingy();

// unsafe because if `classMethod` references `this`, calling
// `classMethod()` won't have `this` bound correctly
const classMethod = thingy.classMethod;

// safe because arrow functions have `this` "pre-bound"
const propertyThatIsAFunction = thingy.propertyThatIsAFunction;

This doesn't just apply to classes, but also to objects, i.e. the above would be exactly the same with an object in place of the class & class instantiation:

const thingy = {
  classMethod() {},
  propertyThatIsAFunction: someFunction,
};

So, when this this rule flags something you are doing as an error, you can create an arrow function:

const classMethod = () => thingy.classMethod();

or bind it when saving it off:

const classMethod = thingy.classMethod.bind(thingy);

or if you have access to the original class and it seems to make sense, convert the method itself in the class definition to an inline arrow function:

class Thingy {
  readonly classMethod = () => {};
}

// or

const thingy = {
  classMethod: () => {};
}

`any`-related linting

One of the risks of using values typed as any is that they can "leak" out into a much bigger footprint of code than it seems like. For example,

// Suppose `create-object` is typed so that `createObject()` returns
// `{ value: string }`
import createObject from 'create-object';

let object = createObject();
// fine
object.value;
// type error
object.subObject.value;

Now, if create-object lacked type information,

// @ts-expect-error no types
// createObject is typed as `any`
import createObject from 'create-object';

// `object` is typed as `any`
let object = createObject();
// fine
object.value;
// also fine at compile-time, but will throw an error at runtime
object.subObject.value;

Basically, as soon as you have a value typed as any, any values you derive from it (via property accesses, function calls, etc etc) will be any and the compiler won't be able to do any meaningful type checking on those values.

We've had @typescript-eslint/no-explicit-any enabled for quite some time, but this just prevents us from explicitly typing a value as any. There are a bunch of cases where a value can be implicitly typed as any, and this rule is meant to "contain the damage."

There a many many cases where this rule flags errors, and many ways of addressing those errors, but here are some common ones I ran into:

inherently untyped values

When calling JSON.parse() or await fetchResponse.json() or the like, there is no static type information -- we, the programmer, have to tell the compiler what kind of value we expect to get from deserializing the JSON string into an object. So here, we just have to cast it, e.g. let myResult = JSON.parse(str) as MyResult;. Sometimes the type isn't defined as a named type and it doesn't seem worthwhile to create a named type, so you can just do let { value } = JSON.parse(str) as { value: string };.

catch blocks

In JavaScript, you can throw anything, not just Error objects. throw 'whoops' or even throw undefined are perfectly valid (although bad practice, and we have a linting rule to prevent us from doing it in our code). So in catch blocks, even though we generally treat the thrown value as an Error, we don't actually know what it is.

Up until recently in our TypeScript configuration (where we had useUnknownInCatchVariables disabled), variables in catch blocks were typed as any (mainly to ease the transition of code from un-typed JavaScript where we play fast and loose with our types to TypeScript). So this new rule flags pretty much any (hah!) usage of the caught variable as an error, and requires an explicit cast. NB, once this was done it wasn't much additional work to enable useUnknownInCatchVariables, which I did, so now these variables in catch blocks are typed as unknown and the compiler (as opposed to the linter) won't let us do anything with them without casting/type-narrowing.

To be as risk-averse as possible, we should ideally always use instanceof checks or type guards or whatever to ensure that the thrown thing is of a type that is compatible with what we expect, e.g.

try {
  doThing();
} catch (e) {
  if (e instanceof Error) {
    reportErrorMessage(e.message);
  } else if (typeof e === 'string') {
    reportErrorMessage(e);
  } else {
    reportErrorMessage('unknown error');
  }
}

But this is a big pain, and given that we lint against throwing non-Error objects for our code, it should generally be an edge case, so in cases like the above I pretty much always did something like

try {
  doThing();
} catch (e) {
  reportErrorMessage((e as Error).message);
}

and I think that's fine for us to do in general unless we're in an area of the code where we're particularly worried about what might be thrown (e.g. from external code) or want to be extra defensive.

error-typed and poorly-typed values

By "error-typed values" I mean ones where we use @ts-expect-error to tell the compiler to not worry about the fact that we're doing something it doesn't understand, and then it types any values that come out of that statement as any. By poorly-typed values I mean places where we intentionally use "any" such as the context in the ModalManager (typically because we haven't yet taken the effort to figure out how to type it correctly).

In both of these cases, if you can't fix the underlying problem, just cast. For example,

// @ts-expect-error no types
import createObject from 'create-object';

let obj = createObject() as {
  key: string;
  stats: { count: number }
};
let count = obj.stats.count;

or even (although the former is probably preferable in most cases)

// @ts-expect-error no types
import createObject from 'create-object';

// leave it typed as `any`
let obj = createObject();
// we don't care about `key` here, so don't bother to include it
let count = (obj as { stats: { count: number } }).stats.count;

promise/async-related linting

We now have linting rules that verify a bunch of things related the async/await and promises, and force us to be more thoughtful and intentional about our async/promise handling, which produces safer code. The two most significant rules are:

@typescript-eslint/require-await

This rule requires that async functions either have an await statement in them, or explicitly return a promise. When this rule flags an error, you probably want to just remove the async keyword from the function, making it synchronous. However, in some cases you might be passing the function to an API that requires a promise-returning function, so just removing the async keyword produces a type error. In this case you can leave the async and wrap any return values in Promise.resolve() e.g. return Promise.resolve('result') or just return Promise.resolve() if the function returns void.

@typescript-eslint/no-floating-promises

This rule prevents us from "ignoring" promises. For example, if you call an async function, you have to do one of the following:

// await the returned promise
await asyncThing();
// store the promise in a variable
const promise = asyncThing();
// pass the promise to a function
handlePromise(asyncThing());
// apply a `.catch()` handler to it
asyncThing().catch((e) => /* ... */);
// explicitly ignore it with the `void` operator
void asyncThing()

This is meant to ensure that we don't accidentally discard promises that we should be handling, both to prevent async/race condition/timing errors, and to make sure that if the promise rejects, it isn't an unhandled rejection (which can cause us to miss errors or, in Node, to cause the process to exit).

This rule forces us to think more carefully about our use of asynchronous functions, and whether they are actually asynchronous at the API level, or whether they just have asynchronous behavior as an internal implementation detail. Consider:

async function getFromServer(url: string) {
  let response = await fetch(url);
  return response.json();
}

async function setAndTrack(obj: Thing) {
  obj.value = 'hello';
  // get some stats from the object (async operation) and send them to a metrics service
  let stats = await obj.getStats();
  await sendTracksToMetricsService(stats);
}

In the first case, the caller would await the function call, and the async-ness is actually a part of the function's API -- getting from the server is inherently asynchronous and the caller needs to wait for the result. In the second case, the caller would not await the function call because metrics tracking is a side-effect and the fact that it's asynchronous is an implementation detail. So the second function's API is synchronous, we are just making the function async so we get the convenient await syntax, rather than having to use promise chaining:

function setBroadcast(obj: Thing) {
  obj.value = 'hello';
  // get some stats from the object (async operation) and send them to a metrics service
  obj.getStats().then((stats) =>
    sendTracksToMetricsService(stats)
  ).catch((e) => /* handle error */);
}

So, if we had the original async implementation of setBroadcast, it would be correct to call it without awaiting it because the caller doesn't want to wait for the metrics tracking to complete before proceeding. But this would trigger a linting error. In this case, it's telling us that the function's API should really be synchronous, and we should rewrite the function to not be async (rather than having all the call sites add a void or catch()).

In this case, we can have our cake and eat it too using an async IIFE:

function setBroadcast(obj: Thing) {
  obj.value = 'hello';

  (async () => {
    // get some stats from the object (async operation) and send them to a metrics service
    let stats = await obj.getStats();
    await sendTracksToMetricsService(stats);
  })().catch((e) => /* handle error */);
}

In cases where we truly do want to invoke an async function but not wait for it to complete before proceeding, it's probably best to

doAsyncThing().catch(captureException);

or in very rare cases where we're pretty darn confident it's not going to throw an error

void doReallySafeAsyncThing();

Embroider: from zero to route splitting in 3.5 weeks

Ben Demboski — Wed, 19 May 2021 06:07:56 +0000

I spent the last 3.5 weeks or so switching our primary app over to using embroider, and getting it working with all the optimized settings plus code splitting across routes. Hopefully reading about my experience will help others through the same process, and help accelerate polishing and adoption of embroider within the Ember ecosystem.

Background
- Our application
- Goals
- Pre-embroider solutions
Our experience moving to embroider
- Overall impressions
- Process
- What was hard
Other tips, tricks, and gotchas
- Addon build failures
- Out-of-memory errors in CI
- Addon-supplied webpack configuration
- Rebuild on addon changes
Conclusions

Background

Our application

I work at Rowan Patents, and our main Ember app is a desktop (ember-electron) application for patent attorneys to use to draft patent applications -- somewhat analogous to an IDE like VSCode. Most patents consist of a bunch of pages of prose, and some pages of drawings, so most patent attorneys use Microsoft Word and Visio for their patent drafting.

Rowan Patents brings together the prose and drawings, along with a number of other efficiency tools, into a single multi-window desktop application. When an attorney opens a patent application file, they will have their primary prose window, and then also might simultaneously open a drawings window and a tools window (and there are several other special-purpose windows like a splash screen and a landing page). This means that there's a pretty strong association between routes and windows -- the prose window will never visit any of the drawings routes, and vice versa -- and also that we typically have several copies of the application loaded into memory at a time (one per window).

We also integrate quite a few third-party libraries for our core editor technology, for our core diagramming technology, and various other data analysis and transformations such as natural language processing. We've found that many of these bundle a lot of extra code that we don't actually execute or need.

Statistics on our app (line counts done using cloc):

11 addons
~55,000 lines of Ember js/ts code
~9,500 lines of hbs
~3,500 lines of scss

Goals

Our two primary goals in moving to embroider were to be able to take advantage of tree-shaking and code splitting across routes, to keep our code-size-related memory footprint under control without incurring high up-front engineering costs or ongoing maintenance costs.

Pre-embroider solutions

Before moving to embroider, we were able to do quite a bit of "manual tree-shaking" of third-party non-Ember libraries. Using module aliasing, null-loader, and string-replace-loader in the webpack config that we supplied to ember-auto-import, we were able to strip out a lot of unneeded code, but at the cost of significant up-front effort and maintenance cost as we upgrade those third-party libraries.

We did not have a solution for splitting our code across routes.
We considered Ember Engines, but opted to wait for embroider to mature...which it has!

Our experience moving to embroider

Note: all of this was using embroider v0.40.0.

Overall impressions

Based on my overall experience, I would qualify embroider as late-stage alpha or early-stage beta quality software. The core functionality is there and is solid, but I still ran across several fairly severe bugs, and the documentation is limited. I spent a good deal of time debugging through embroider's code to determine why certain aspects of my build weren't working, and I don't think I would have successfully executed the whole transition to embroider if I had been unwilling to roll up my sleeves and dive in like that. I don't think the same is necessarily true of smaller apps with fewer dependencies though.

However, after getting our application building, and fixing some runtime errors, it's been feeling stable, and I'm not feeling nervous about inconsistencies in production build output, frequent but intermittent oddities in the development environment, or any of those other things that would otherwise be keeping me up at night.

Process

My process for getting our app/addons building on embroider with optimized settings was:

Get all addons building on embroider
Get the app building on embroider
Manually test & stabilize the app
Merge back to main
Get all addons building with optimized flags
Get the app building with optimized flags
Enable route code splitting
Manually test & stabilize the app
Merge back to main
Celebrate, spend a chill weekend backpacking on the Washington Coast, and then write this article

What was hard

I've tried to break down all the work I put into making the app build on embroider into categories, and I'll describe them generally without diving too deep. Hopefully they make some sense and it's useful -- I promise nothing!

ES6 module compliance

One thing we had to update in a number of places was code that imported a module as if it had a default export when really it only had named exports. For example, given a module named lib with two named exports, foo and bar (and no default export), we would sometimes do this:

import lib from 'lib';

lib.foo();
lib.bar();

This would work with Ember's module loader, but is not ES6-compliant, and will not work with embroider/webpack. We had to either update to:

import * as lib from 'lib';

lib.foo();
lib.bar();

or ideally (because it's better for tree-shaking):

import { foo, bar } from 'lib';

foo();
bar();

Another related issue had to do with export stubbing. We had a relatively common pattern in our tests of using sinon to stub or spy on module exports. Using the above lib example, we might have a test that would do:

let fooStub = sinon.stub(window.require('lib'), 'foo');
fooStub.returns({ result: 'value' });

await click('.button-that-calls-lib-foo');
assert.equal(fooStub.callCount, 1);

The window.require() would use Ember's module loader to load the foo module, and then stub out one of its exports. This will not work with webpack-generated modules for a couple of reasons, so we had to find other patterns, usually involving either doing the stubbing at a different point in the call stack, or instrumenting the logic in the code itself to provide a stubbing mechanism that wasn't re-writing the ES6 module objects themselves.

package.json dependencies

Unfortunately I don't have a lot of information to share on this one because I never fully bottomed out my understanding, just treated symptoms. But I'll describe what I do know.

We had a number of build failures related to transitive dependencies. For example, we use ember-power-select, which depends on ember-basic-dropdown, and also use ember-basic-dropdown directly. We did not have ember-basic-dropdown declared in our package.json, so when @embroider/compat assembled our app into the v2 package format, it would not make ember-basic-dropdown accessible to our app (basically it would put it in node_modules/ember-power-select/node_modules/ember-basic-dropdown instead of node_modules/ember-basic-dropdown).

I believe in some cases we ran into errors where the dependency was only listed as a peerDependency and needed to be a dependency or devDependency, although perhaps that was non-addon libraries, I don't recall and unfortunately didn't leave enough of a paper trail to verify.

But overall, given how embroider compiles apps/addons into a more statically analyzable and broadly standards compliant package format, we had to do a fair amount of hardening how we declare our dependencies in our various package.jsons.

Statically analyzable components

This already has a great writeup in the embroider documentation. We weren't doing anything truly dynamic, only using dynamic-looking components as a way of currying components' arguments multiple times, so it was not very difficult for us to refactor to avoid doing that, although in retrospect I think we might have been able to more easily do it using the not-yet-documented (outside code comments, as far as I can tell) packageRules.

CSS/SASS

I spent a good deal of effort getting our SCSS (via ember-cli-sass) and CSS working under embroider. Unfortunately I don't have great detail to offer here, because due to Reasons™ (and probably not great ones), I ended up switching from node-sass to dart-sass as part of the move to embroider, so I'm not actually certain how much fiddling was needed for embroider, and how much for dart-sass. But I am certain that it was a mix of the two, so be warned that there may be some non-trivial effort involved in getting your styles working when moving to embroider.

Third-party addons

Hooboy, this was definitely a biiiiig hunk of the work. Our app has been under development for 6ish years, so it has a whole history of legacy code relying on older addons that aren't really Octane-y, plus several modern addons that haven't yet been brought up to embroider compatibility, let alone embroider optimized compatibility.

Getting all the third-party addons working took a lot of effort, but fortunately webpack provides a lot of configuration and tools to hack on things and make them work. Here is a list of addons that were problematic (that should shrink over time) and what we did about them -- I hope these solutions will help people with identical problems and/or serve as examples to help address issues with addons we do not use.

I should also note that there are undoubtedly better ways of doing some of this -- I often took a brute force webpack approach because that's what I knew, but I believe embroider's packageRules might provide nicer solutions to some of these issues.

Before trying any workarounds, I would always update the problematic addon(s) to their latest version, so all of these descriptions and workarounds apply to latest published versions as of 5/14/2021.

ember-file-upload

ember-file-upload imports code from ember-cli-mirage that might not actually be included in the build -- see the discussion in this issue for more details. Since we don't use ember-file-upload's mirage utilities, we worked around this by stubbing out the import:

const emberFileUploadMiragePath = path.join(
  path.dirname(require.resolve('ember-file-upload')),
  'mirage'
);

// <snip>

webpackConfig: {
  resolve: {
    alias: {
      [emberFileUploadMiragePath]: false,
    },
  },
}

ember-power-select

Despite having some code in @embroider/compat to support this addon, I found that it would not work with the embroider optimized flags all turned on, I think because even if embroider knows what @someComponent is, the or in {{component (or @someComponent "default-component")}} messes up its static analysis. So, I got my friend (that we'll see a lot more of), string-replace-loader to help out and came up with this wonderful bit of webpack config that definitely doesn't make me secretly feel dirty and/or naughty.

ember-concurrency

With embroider optimized flags (staticAddonTrees in particular), production builds of ember-concurrency fail with a bizarre error. The current theory is that it's a webpack bug, but fortunately the workaround is pretty straightforward.

ember-cli-page-object

ember-cli-page-object has a compatibility module that is meant to support older setups with old versions of @ember/test-helpers, or even from before that library existed. It exposes an API that is meant to more or less emulate @ember/test-helpers, and internally does some tricks with testing for modules at runtime that does not work with embroider optimized.

Since we know our code always has @ember/test-helpers present, we can use string-replace-loader to make that compatibility module statically import and re-export @ember/test-helpers.

ember-metrics

ember-metrics relies on some features of Ember's container and module loader that do not work under embroider optimized to dynamically look up its metrics adapters based on names specified in config/environment.js. Since I know the static list of adapters that our application uses, I wrote an initializer to statically import those adapters and then register them in the container using the names under which ember-metrics expects to find them:

import intercomAdapter from 'ember-metrics/metrics-adapters/intercom';
import mixpanelAdapter from 'ember-metrics/metrics-adapters/mixpanel';

export function initialize(application) {
  application.register('metrics-adapter:intercom', intercomAdapter);
  application.register('metrics-adapter:mixpanel', mixpanelAdapter);
}

export default {
  initialize,
};

ember-changeset-validations

The whole ember-changeset/ember-changeset-validations/ember-validators bundle did not have dependencies set up and declared in a way that would work with embroider optimized, so some module aliasing and an appearance by my new best friend, string-replace-loader provided a workaround.

ember-validated-form

ember-validated-form does a lot of dynamic component invocations that embroider cannot statically analyze. I believe the fact that it uses curly component syntax and doesn't use this.foo vs @foo makes it even harder on embroider. The workaround involved soooooo much string-replace-loader, but it works!

Route-splitting miscellanea

At the end of this all, enabling route-splitting was a very pleasant anti-climax. It almost just worked, and I was SO EXCITED to see only the code I expected/wanted loaded in each window of our application. The couple of minor issues I had to address when enabling route splitting were:

We had one route whose dynamic parameter was :id and even though it didn't need to implement the serialize() hook (which is warned about in the documentation), when using the embroider router, passing a model instance to RouterService.urlFor() did not work. Evidently the code wanted the dynamic parameter to end with _id before it would look for an id property on the model instance, so I changed the dynamic parameter to :sheet_id and everything was hunky dory
We had one test that was doing this.owner.lookup('controller:invention') before calling visit(), and that was not returning the controller instance. I believe this was because that controller/route were lazy-loaded, so the code wasn't loaded and the factory wasn't registered with the container before visit(). I fixed this by shuffling the test code around to not do the lookup() until after the visit() call, but this one has me kinda nervous that I have a lot of other lookup()s like this that are only succeeding because they run after tests that have already caused the code for the lazy-loaded routes to load...but I haven't confirmed/investigated this any further yet.

Other tips, tricks, and gotchas

Here I've collected some gotchas that I ran into and overcame, and some tips and tricks for smoothing out the whole setup and developer experience of working in this bravely-embroidered new world.

Addon build failures

There is currently a bug in embroider that causes addon builds to non-deterministically produce incorrect output so that tests and the dummy app will not load or run. Until it is fixed, the workaround is to disable concurrency (set JOBS=1) in your environment anytime you're building addons (this does not have any effect on apps).

Because this not only produces a broken build but also poisons the embroider cache, I've applied a draconian workaround to all of our addons:

// ember-cli-build.js
'use strict';

const EmberAddon = require('ember-cli/lib/broccoli/ember-addon');
const { Webpack } = require('@embroider/webpack');

module.exports = function (defaults) {
  let app = new EmberAddon(defaults, {
    // Add options here
  });

  // https://github.com/embroider-build/embroider/issues/799
  process.env.JOBS = '1';

  return require('@embroider/compat').compatBuild(app, Webpack);
};

Out-of-memory errors in CI

Until this fix is released (which should happen in the next release after v0.40.0), embroider doesn't quite fully respect JOBS=1. With JOBS=1 it will run the entire build in a single process, but it still warms a worker pool whose size is number-of-cpus-minus-one which, in CI, can be a very big number. Spawning all of these processes and having them load up their code (even though they don't do anything) consumed enough memory that in some of our CI scenarios, there was not enough left over for our build/tests/etc.

I don't know of a good workaround. My super ugly one was to compile @embroider/webpack's latest master, check in a copy of ember-webpack.js to my source tree, and then manually copy it into node_modules to overwrite the existing v0.40.0 file at the beginning of the CI run. DON'T LOOK AT ME!!!!!

Addon-supplied webpack configuration

When using ember-auto-import pre-embroider, addons could supply a configuration to use when building their assets, including a webpack configuration. This could be very handy when addons included third-party libraries that they themselves added to the application bundle using ember-auto-import. As far as I can discern, embroider doesn't have a similar mechanism.

I think the whole idea of modularizing and then merging webpack configs is a little fraught because webpack wasn't really designed for modular configurations -- it all gets merged into a single configuration, so it's not hard to concoct scenarios where the different "modules" of configuration conflict and mess each other up.

That being said, in controlled environments like ours where the addons are private to our applications, and we have specific knowledge of how/where they will be used, such a mechanism can be very handy. So we ended up standardizing on addons putting their webpack configuration in /webpack.config.js so they could be imported and merged from consuming apps/addons.

For example, suppose we have an app, creatively called app that depends on addon-a and addon-b. This gist shows how we might put together and consume reusable webpack configs for this scenario.

Rebuild on addon changes

Pre-embroider, if an addon's index.js implemented isDevelopingAddon() to return true, then when running ember serve or ember test -s, the build watcher would watch that addon and auto-rebuild/live-reload when any changes were made to that addon. embroider uses a different mechanism, which is the environment variable EMBROIDER_REBUILD_ADDONS that can contain a comma-separated list of the names of addons to watch.

I like this mechanism much better because I don't have to modify code to change which addons are and aren't build-watched. But in the specific case of our monorepo with our app and addons, I always want to build-watch the in-monorepo addons. This is because our addons are not published, and are really only separate from the app for code organization and dev/test efficiency reasons. I could set EMBROIDER_REBUILD_ADDONS to those addons in my login configs or something, but I wanted a zero-config solution that would work for my whole team. So I settled on doing this little trick in all our in-monorepo addons' index.js files:

module.exports = {
  name: require('./package').name,
};

// embroider doesn't respect isDevelopingAddon(), and we don't publish this
// addon, so always add ourselves to the list of auto-rebuild addons
process.env.EMBROIDER_REBUILD_ADDONS = Array.from(
  new Set([
    ...(process.env.EMBROIDER_REBUILD_ADDONS || '').split(','),
    require('./package').name,
  ])
).join(',');

This way anytime an app or addon runs a build that depends on this addon, it will add itself to the list of rebuild addons.

PLEASE DO NOT DO THIS IN PUBLISHED ADDONS!!!

Conclusions

At the end of this all, we had what we wanted (tree-shaking and code splitting across routes), and I was very excited. It was a good deal of work (3.5 weeks or so as my main focus), although a lot of that was taken up by investigations of embroider bugs, followed by bug reports, sometimes contributing fixes, and sometimes finding the workarounds described above. So hopefully that will pave the way for a faster/smoother experience for others.

embroider is moving fast, and I'm sure much of this article will be obsolete before long, but I hope it does provide some value in the meantime. I'd also love to hear from others -- your experiences with embroider, ideas to improve on the suggestions and methods I've provided, core team members telling me how I've missed the boat and misrepresented anything/suggested anything silly, etc. I hang out in the #dev-embroider channel on Ember Discord, and will happily discuss/answer questions/etc. here or there.

Simplifying the git forking workflow

Ben Demboski — Tue, 13 Apr 2021 18:01:36 +0000

The standard way to contribute to an open source project that you do not maintain is to fork it, create a branch in your fork where you put your code changes, and then open a pull request into the original repository. I've been doing this for years, and just discovered a tweak to the workflow that I really like and want to share with you.

The git forking workflow

This is a well established pattern that has been written up a number of times (e.g. here and here), so I'll just briefly outline the process of opening a pull request, and then opening a second pull request, as I originally learned it.

First pull request

To open my first pull request, I need to first create a fork. I'll use the Ember test helpers repo as my example that I'm contributing to. Note that none of this is specific to GitHub -- it would work the same with BitBucket or any other git host. My steps are:

Fork the repo, so the original is at git+ssh://git@github.com/emberjs/ember-test-helpers and my fork is at git+ssh://git@github.com/bendemboski/ember-test-helpers.
git clone git+ssh://git@github.com/bendemboski/ember-test-helpers
git checkout -b my-branch-1
Write code
git push -u origin my-branch-1

and now I'm ready to open a pull request! This is nice and simple, but where I think it gets a little complicated is when I want to make my second pull request.

Second pull request

If some time has passed, Ember test helpers' master branch will have changed since I created my fork as work on the project continues -- at the very least, I hope my pull request was merged into it! So when creating my new branch, I need to make sure to branch off of the latest master in the original repo, not the out-of-date one in my fork.

The steps are:

git remote add upstream git+ssh://git@github.com/emberjs/ember-test-helpers
git fetch upstream master
git checkout master
git merge upstream/master
git checkout -b my-branch-2
Write code
git push -u origin my-branch-2

Steps 1-4 are just syncing my local mirror of my fork's master branch with the master branch in Ember test helpers' repo. Since I don't actually do anything with my fork's master branch, I could simplify this slightly:

git remote add upstream git+ssh://git@github.com/emberjs/ember-test-helpers
git fetch upstream master
git checkout -b my-branch-2 upstream/master
Write code
git push -u origin my-branch-2

Simplified git forking workflow

The simplification involves a tweak to the workflow that is pretty minor from a mechanical/what-commands-do-I-type standpoint, but I think simplifies the mental model significantly.

First pull request

Instead of cloning my fork of the repo, I will clone the original repo and then add my fork as another remote:

Fork the repo, so the original is at git+ssh://git@github.com/emberjs/ember-test-helpers and my fork is at git+ssh://git@github.com/bendemboski/ember-test-helpers.
git clone git+ssh://git@github.com/emberjs/ember-test-helpers <-- this is the key difference
git checkout -b my-branch-1
Write code
git remote add bendemboski git+ssh://git@github.com/bendemboski/ember-test-helpers
git push -u bendemboski my-branch-1

The server-side result is the same -- my fork has a my-branch-1 branch ready to use for a pull request into the original repo, but the local setup is different in a way that makes opening subsequent pull requests somewhat simpler.

Second pull request

Since I have cloned the original repo, I sync my local master branch just like I would with any other branch, simplifying the beginning of this workflow:

git checkout master
git pull
git checkout -b my-branch-2
Write code
git push -u bendemboski my-branch-2

Practical differences

The only practical difference between these two versions of the forking workflow is that in the simplified form I'm not trying to keep my fork's master up-to-date with the original repo's. In fact, I completely ignore my fork's master branch and just treat my fork as a repository for pushing temporary branches to support opening pull requests.

Ergonomic benefits

Even though the pure number of commands I need to type isn't significantly reduced, in my experience, this simplifies the mental model in a way that reduces friction in the whole process of opening pull requests. The benefits I've experienced are:

I don't have to worry about whether my fork's master branch is up-to-date with the original repo's master branch
I don't have to worry about accidentally merging code into my fork's master branch in a way that would require something like rebase to get back in sync with the original repo's master branch
I don't have to think about the fact that I'm working with a fork of a repository aside from the one time (per branch) that I have to push my branch to the remote pointing to my fork (git push -u bendemboski ... instead of git push -u origin ...). All of my pulling and branching operations are done just as if I owned the repository, and it's only the first time I push a branch that I have to do something different.

These may not seem like a huge deal, but for me they are, because of the mental simplification of not having to switch between two different "modes" -- the "working on an original repo" mode and the "working on a fork of a repo" mode. When I'm working in my local clone, it makes no difference and I do the same thing either way, and it's only when I need to push a new branch to somewhere remote that I have to think about the difference, and that's exactly when I should be thinking about the difference!

This peels off one extra layer of mental load and reduces the friction involved in the whole process.

An extra thought

There can be good reasons to use the forking workflow even for repositories that you can push to, e.g. to keep from polluting the original repository with experimental branches, etc. As the ever-insightful @katiegengler pointed out in a Discord discussion, in such cases following the simplified forking workflow but cloning the original repo using the https instead of git URL (git clone https://github.com/emberjs/ember-test-helpers instead of git clone git+ssh://git@github.com/emberjs/ember-test-helpers) adds an extra layer of protection preventing you from accidentally pushing to the original repo instead of your fork.

Conclusion

I've found this tweak to the workflow to be a non-trivial simplification that noticeably improves my developer experience of periodically contributing to projects that I don't own. I love the open source model, and I love contributing back to projects that I have benefited from, so I'm always excited to find ways of reducing friction in the process to make me more likely to do it, and free up energy for the actual development work that's fun rather than the git mechanics that are...less fun. I'd love to hear what you think about this simplified workflow, or other ways you've found to reduce friction in contributing to open source projects.

DEV Community: Ben Demboski

Upgrading to Ember 6.1: linting with type information

New linting setup

any-related linting

inherently untyped values

catch blocks

error-typed and poorly-typed values

promise/async-related linting

Embroider: from zero to route splitting in 3.5 weeks

Background

Our application

Goals

Pre-embroider solutions

Our experience moving to embroider

Overall impressions

Process

What was hard

ES6 module compliance

package.json dependencies

Statically analyzable components

CSS/SASS

Third-party addons

Route-splitting miscellanea

Other tips, tricks, and gotchas

Addon build failures

Out-of-memory errors in CI

Addon-supplied webpack configuration

Rebuild on addon changes

Conclusions

Simplifying the git forking workflow

The git forking workflow

First pull request

Second pull request

Simplified git forking workflow

First pull request

Second pull request

Practical differences

Ergonomic benefits

An extra thought

Conclusion

`any`-related linting