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

GN-4800: Bump appuniversum to v3 #645

Merged
merged 2 commits into from
Apr 25, 2024
Merged

GN-4800: Bump appuniversum to v3 #645

merged 2 commits into from
Apr 25, 2024

Conversation

dkozickis
Copy link
Contributor

@dkozickis dkozickis commented Apr 23, 2024

Overview

GN-4800: Bump @appuniversum/ember-appuniversum to 3.4.1

  • Bump @appuniversum/ember-appuniversum to 3.4.1

    • Update breaking changes, mainly removing two-way binding from AuTextarea and AuInput components.
    • Change how appuniversum SCSS is imported in the app.
    • Add a key to AuDatePicker localisation.
  • Bump @lblod/ember-rdfa-editor-lblod-plugins to 17.0.0

  • Bump @lblod/ember-environment-banner to 0.5.0

connected issues and PRs:

Setup

  1. Checkout
  2. npm i

How to test/reproduce

  • Start GN ember s --proxy https://dev.gelinkt-notuleren.lblod.info
  • Navigate around the app, make sure everything still works, try to input/change input of input field and textareas that you see, as those were the main changes.

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially regarding relationships)
  • changelog
  • npm lint
  • no new deprecations

@dkozickis
Copy link
Contributor Author

dkozickis commented Apr 23, 2024

Converted to Draft as dependency-lint is failing, will work on it .

P.S. Ready for review again

@piemonkey
Copy link
Contributor

@dkozickis I think dependency lint is just because there's a new peer-dep for AU >3.

I think that there's more to this upgrade though than this, unfortunately the problems will only come up by either testing every template in the app or searching for deprecated uses of components, since we don't have TS and glint to lint our templates.

When doing this in the plugins, I looked at the release notes of AU and checked the usage of the components. For example v2.13.0 includes this deprecation of @value on <AuInput>, which I can see still exists in app/components/zitting/manage-zittingsdata.hbs.

@dkozickis
Copy link
Contributor Author

For example v2.13.0 includes this deprecation of @value on <AuInput>, which I can see still exists in app/components/zitting/manage-zittingsdata.hbs.

@value is deprecated and removed in v3, instead value (no @) is used, with change handler.

@dkozickis dkozickis marked this pull request as ready for review April 23, 2024 13:12
@piemonkey
Copy link
Contributor

@dkozickis Ah, sorry, when I looked at the PR it was just the dependency change, none of the code changes, I guess I caught it when not all the commits were in.

Copy link
Contributor

@piemonkey piemonkey left a comment

Choose a reason for hiding this comment

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

LGTM, just one comment. Happy for it to be merged after that or if there's a good reason not to do it.

package.json Outdated
@@ -29,7 +29,7 @@
"release": "release-it"
},
"devDependencies": {
"@appuniversum/ember-appuniversum": "~2.15.0",
"@appuniversum/ember-appuniversum": "~3.4.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I think we can go for ^3.4.1 as AU versions follow semver pretty well.

@@ -111,7 +111,9 @@
},
"overrides": {
"@appuniversum/ember-appuniversum": {
"tracked-toolbox": "^2.0.0"
"tracked-toolbox": "^2.0.0",
"ember-concurrency": "^2.3.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to override this rather than upgrading? I tested this PR with ^3.1.0 and it seems to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've preferred to override to 2.3.7 because all other dependencies have it, below is the output of ember dependency-lint, I will crate tech tasks to update ember-concurrency everywhere else 🤔

ember-concurrency
Allowed: (any single version)
Found: 3.1.1, 2.3.7
frontend-gelinkt-notuleren
├── [email protected]
├─┬ @appuniversum/ember-appuniversum
│ └── [email protected]
├─┬ @lblod/ember-mock-login
│ └── [email protected]
├─┬ @lblod/ember-rdfa-editor-lblod-plugins
│ └── [email protected]
└─┬ ember-power-select
  └── [email protected]

@dkozickis dkozickis merged commit 3ed6ea7 into master Apr 25, 2024
3 checks passed
@dkozickis dkozickis deleted the GN-4800 branch April 25, 2024 08:09
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.

2 participants