CrowdStrike / ember-headless-table

https://ember-headless-table.pages.dev
MIT License
23 stars 7 forks source link

Update ember-resources from v5 to v7 #281

Open NullVoxPopuli opened 1 month ago

NullVoxPopuli commented 1 month ago

Via

npx ember-resources-codemod

Here is the code for the codemod: https://github.com/NullVoxPopuli/ember-resources/pull/1147


There aren't many breaking changes from v5 to v7.

Here are the breaking changes in v6:

Here are the breaking changes in v7:


Of note, because ember-resources has such broad support (3.28 - 6+), ember-resources declares a peer on ember-source so that it can correctly choose which features to use based on the consuming app's version.

As a result, ember-resources (in pnpm7 anyway), ember-resources must also become a peer, which is a breaking change. (However, I'm exploring if peers can be removed entirely here https://discord.com/channels/480462759797063690/568935504288940056/1281690947708784681 -- which would make this PR not a breaking change (once I propagate the findings to the relevant libraries))

There is a fair bit of this code:

import { dependencySatisfies, importSync, macroCondition } from '@embroider/macros';

let getOwner: (context: unknown) => Owner | undefined;
let setOwner: (context: unknown, owner: Owner) => void;

if (macroCondition(dependencySatisfies('ember-source', '>=4.12.0'))) {
  getOwner = importSync('@ember/owner').getOwner;
  setOwner = importSync('@ember/owner') .setOwner;
} else {
  getOwner = importSync('@ember/application').getOwner;
  setOwner = importSync('@ember/application').setOwner;
}

The default ember-source for ember-headless-table is v4, but it looks like 3.28+ is also supported (the same range as ember-resources).

Though, while addressing this, I ran in to a number of peer problems. It may be best to address those in separate PRs, but they are included here in hopes that CI goes green, since I hadn't yet contributed to this repo since my time at CS. image

I also ran in to a number of maintenance problems that will probably not mean this PR will go green

These alone are not problematic except when wanting to have fixed build problems. In particular, I noticed with the current repo's dependencies, the dependency graph is incorrect, and packages are being resolved incorrectly -- for example, something is trying to resolve reactiveweb/dist/index, but that file doesn't exist, and nor should it -- because you only pay for what you import, and having a barrel file for a generic library would be bad for performance.

So, I guess this PR is more demonstration than anything, as other work must happen before this work can attempt to go green.

(If I used this library, I'd PR fixes for ya'll -- but I haven't had a need for a table in a while -- my timebox ran out on trying to get this repo working <3 )

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: 80a14ef50183d55ae06d31b5482af4ce1fae078e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | -------------------- | ----- | | ember-headless-table | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR