Skip to content
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

Update ember-resources #480

Merged
merged 7 commits into from
Sep 28, 2024
Merged

Update ember-resources #480

merged 7 commits into from
Sep 28, 2024

Conversation

piemonkey
Copy link
Collaborator

Overview

Based on PR from NullVoxPopuli, but with master merged, a changeset and minor tweaks.

connected issues and PRs:

The original: #477

Setup

N/A

How to test/reproduce

Anything using ember-resources works, e.g. search boxes. Also should work when linked to GN or RB frontends.

Challenges/uncertainties

This will require an upgrade from consumer apps that use ember dependency-lint, but according to testing, the versions can actually co-exist peacefully. I'm unsure if we should therefore merge this or whether we should wait until we do a major release.

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check if dummy app is correctly updated
  • Check cancel/go-back flows
  • changelog
  • npm lint
  • no new deprecations

Copy link
Collaborator

@elpoelma elpoelma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well 😄
I think we can at least move to the reactiveweb package without issues. Not so sure about the ember-resources package, as we kind of still need to take the app-folder merging into account. I would assume that in this case it uses the host-app version of ember-resources, but I'm not completely sure. Maybe (if we do not want to break the API), we should remain on ember-resources 6.5.1 for now, but already move to reactiveweb.

@piemonkey
Copy link
Collaborator Author

@elpoelma I've tested both (my original version with the ember-resources upgrade and the same with ember-resources not upgraded) and both seem to work fine included in GN as dev versions from npm (both fail with yalc). The only failure I see is ember-dependency-lint, which is being very paranoid. Personally, I think since we control the consuming apps, we can safely upgrade, but I'll change this for the version with ER 6.5 so we can avoid messing with any other consumers.

@piemonkey
Copy link
Collaborator Author

Actually, I just realised that #478 will mean a major release due to dependency changes, so should we remove my latest commit (so we use ER 7.0) and just have an extra dependency change required for that version?

@piemonkey piemonkey force-pushed the nullvox/upgrade-ember-resources branch from f6367f8 to e2f39d5 Compare September 26, 2024 15:58
@elpoelma
Copy link
Collaborator

elpoelma commented Sep 27, 2024

Actually, I just realised that #478 will mean a major release due to dependency changes, so should we remove my latest commit (so we use ER 7.0) and just have an extra dependency change required for that version?

I also have a PR with breaking changes coming up (the snippet rework) so we can add that one too.

Update: #482

@abeforgit abeforgit enabled auto-merge (squash) September 28, 2024 16:22
@abeforgit abeforgit merged commit 789180f into master Sep 28, 2024
2 of 3 checks passed
@abeforgit abeforgit deleted the nullvox/upgrade-ember-resources branch September 28, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants