refactor: convert addon build code to native class syntax

[buschtoens]

trafficstars

As per my message on Discord it seems that native class syntax is actually properly supported by CoreObject and ember-cli.

[dfreeman]

Thanks for the PR, @buschtoens! So far we've intentionally avoided subclassing entities directly from ember-cli because it can put you in a very weird state when working with linked packages.

If ember-cli-typescript is linked (or is a dependency of a package that's linked) to your main project, then the entities it subclasses would be coming from the version of ember-cli in the linked package while being consumed by the version from the host. There are no guarantees that an Addon instance from one version will work when plugged into another (and I know of some changes over the years that would have caused breakage like that), so while it does work to export an actual class here, you can still wind up with odd problems that are hard to debug.

IIRC I had a conversation with someone about this a while back (maybe @pzuraq?) — what we'd really need for this to be safe would be a way to ensure that the base class we're importing is actually the one for the running CLI instance. I could imagine hacking a best-effort version of that with something like this:

function requireLocal(modulePath) {
  try {
    return require(resolve.sync(modulePath, { basedir: process.cwd() }));
  } catch (e) {
    if (e.code === 'MODULE_NOT_FOUND') {
      return require(modulePath);
    } else {
      throw e;
    }
  }
}

export const Addon = requireLocal('ember-cli/lib/models/addon');

There's also project.require, but there's the Catch-22 of that not being available until after your entity has been instantiated.

Neither option seems super appealing to me, but I imagine community desire for using 'real' syntax for these things is only going to grow (particularly if developing build-time code in TS becomes more common), so maybe there's an approach to solving this upstream.

[buschtoens]

Thanks for the detailed explanation! That is indeed a problem and very annoying limitation; not unique to ember-cli though, so there surely must be other projects, which are running into the same issues regarding linked packages. But alas, my Google-Fu is failing me.

I imagine community desire for using 'real' syntax for these things is only going to grow (particularly if developing build-time code in TS becomes more common), so maybe there's an approach to solving this upstream.

Definitely. I feel uncomfortable writing pure JS by now. Unfortunately, the ember-cli-entities workaround comes with its own trade-offs and limitations. :/

[chriskrycho]

I'd add that I at least want to get the heck away from CoreObject before we start trying to move this way. It's not necessary at this point as far as I can tell; it's just hard to drop because of the ecosystem state.

[chriskrycho]

@buschtoens any chance you can make the suggested tweaks here?

[buschtoens]

@chriskrycho Happy to do so, I just wasn't sure whether requireLocal would be a "good enough" solution already for this PR to get merged.

Dropping CoreObject is not a requirement?

[chriskrycho]

We chatted about this, and we still super appreciate the work (three years later 😂) but are going to close it:

• We're working on an RFC to switch ember-cli-typescript into maintenance mode—details forthcoming, but the short version is: the broader ecosystem has moved around us to a default of having folks just run tsc directly as part of dev and CI, while using tools like Babel, ESBuild, SWC, etc. for the transpilation, and keeping those fully decoupled. We want to match that (and as folks move into an Embroider-native world, they will be able to opt into things like ts-loader in webpack if they want errors injected into the build).

• All the points discussed above back when this was opened originally.

Net, we want to minimize further direct investment in ember-cli-typescript internals. Thanks again, @buschtoens!