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

Upgrade add-on to Ember.js v5.4.0 #255

Merged
merged 10 commits into from
Nov 12, 2023
Merged

Conversation

MrChocolatine
Copy link
Member

@MrChocolatine MrChocolatine commented Oct 29, 2023

Breaking

Upgrade add-on to Ember.js v5.4.0 (#255)

  • Ember.js v4.8 or above is now required
  • Ember CLI v4.8 or above is now required
  • Node.js v18 or above is now required

Chore

Partially migrate add-on to TypeScript (#255)

Build

Dependencies updates (#255)

Added

  • @babel/core@^7.23.2
  • @ember/string@^3.1.1
  • @glint/environment-ember-loose@^1.2.1
  • @glint/template@^1.2.1
  • @typescript-eslint/eslint-plugin@^6.9.1
  • @typescript-eslint/parser@^6.9.1
  • concurrently@^8.2.2
  • ember-cli-clean-css@^3.0.0
  • rimraf@^5.0.1
  • stylelint@^15.11.0
  • stylelint-config-standard@^34.0.0
  • stylelint-prettier@^4.0.2
  • typescript@^5.2.2
  • @ember/string@^3.1.1 is now a peer dependency
  • ember-source >= 4.0.0 is now a peer dependency

Updated

  • ember-auto-import@^2.6.3 is now a dependency, not a dev dependency
  • ember-cli-babel@^8.2.0
  • ember-cli-htmlbars@^6.3.0
  • ember-local-storage@^2.0.6
  • ember-truth-helpers@^4.0.3
  • @ember/test-helpers@^3.2.0
  • @embroider/test-setup@^3.0.2
  • @glimmer/component@^1.1.2
  • @glimmer/tracking@^1.1.2
  • ember-auto-import@^2.6.3
  • ember-cli@~5.4.0
  • ember-cli-dependency-checker@^3.3.2
  • ember-page-title@^8.0.0
  • ember-qunit@^8.0.1
  • ember-resolver@^11.0.1
  • ember-source@~5.4.0
  • ember-template-lint@^5.11.2
  • ember-try@^3.0.0
  • eslint@^8.52.0
  • eslint-config-prettier@^9.0.0
  • eslint-plugin-ember@^11.11.1
  • eslint-plugin-n@^16.2.0
  • eslint-plugin-prettier@^5.0.1
  • eslint-plugin-qunit@^8.0.1
  • prettier@^3.0.3
  • qunit@^2.20.0
  • webpack@^5.89.0

Removed

  • @babel/core@^7.18.10
  • @babel/eslint-parser@^7.18.9
  • @babel/plugin-proposal-decorators@^7.18.10
  • ember-disable-prototype-extensions@^1.1.3
  • ember-export-application-global@^2.0.1
  • npm-run-all@^4.1.5

Below is just extra text to give more context, but it should not be included in our changelog.

Actions taken

  1. Fresh add-on created using the latest version of Ember.js + flag --typescript (using this brings additional changes compared to a simple ember new MyAddon)
  2. Then re-import everything from our add-on
  3. Re-adjust a few things to make it work for our setup

ndekeister-us
ndekeister-us previously approved these changes Oct 30, 2023
Copy link
Member

@ndekeister-us ndekeister-us left a comment

Choose a reason for hiding this comment

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

LGTM

@MrChocolatine
Copy link
Member Author

Ember.js v5.4.0 has just been released, I will apply the change here while I am at it.
emberjs/ember.js@v5.4.0 (release)

@MrChocolatine MrChocolatine marked this pull request as draft October 30, 2023 19:08
@MrChocolatine MrChocolatine changed the title Upgrade add-on to Ember.js v5.3.0 Upgrade add-on to Ember.js v5.4.0 Nov 4, 2023
@MrChocolatine MrChocolatine dismissed ndekeister-us’s stale review November 4, 2023 13:08

New review requested: I applied another upgrade (v5.3.0 to v5.4.0)

@MrChocolatine MrChocolatine force-pushed the upgrade-to-emberjs-5.3.0 branch 4 times, most recently from 4316c00 to c2cb36a Compare November 4, 2023 13:37
@MrChocolatine MrChocolatine marked this pull request as ready for review November 4, 2023 13:41
@MrChocolatine MrChocolatine force-pushed the upgrade-to-emberjs-5.3.0 branch 4 times, most recently from 10a9303 to 2648586 Compare November 5, 2023 22:34
@MrChocolatine
Copy link
Member Author

Calling a last review @ndekeister-us , I dismissed your last approval as I applied another upgrade (v5.3.0 to v5.4.0) after it.

BREAKING CHANGE: upgrade add-on to Ember.js v5.4.0

- Now require Ember.js v4.8 or above
- Now require Ember CLI v4.8 or above
- Now require Node.js v18 or above
There were some warnings when running `npm publish --dry-run`:

```
➜  npm publish --dry-run
npm WARN publish npm auto-corrected some errors in your package.json when publishing.  Please run "npm pkg fix" to address these errors.
npm WARN publish errors corrected:
npm WARN publish "repository" was changed from a string to an object
npm WARN publish "repository.url" was normalized to "git+https://github.com/DazzlingFugu/ember-feature-controls.git"
npm notice

... logs ...
```
Otherwise we get this error when runnning tests:

```
yarn test:ember

$ ember test
Environment: test
cleaning up...
Cleanup error.
⠋ cleaning upCannot read properties of undefined (reading 'cleanup')

Stack Trace and Error Report: /Applications/code-portable-data/tmp/error.dump.c09e9dbb0ebb48059d2c311cec4335fa.log
ember-feature-controls needs to depend on ember-auto-import in order to use ember-truth-helpers

Stack Trace and Error Report: /Applications/code-portable-data/tmp/error.dump.dac3bb11d1b83deb32927642f21cb0f4.log
error Command failed with exit code 1.
```
Prior to these changes, we were getting this error:

```
Unexpected use of `assert.expect()` call. (eslintqunit/require-expect)
```

from https://github.com/platinumazure/eslint-plugin-qunit/blob/master/docs/rules/require-expect.md
1. The coment says this override can be removed once mansona/ember-get-config#29
   has been merged, and it was merged.
2. `ember-get-config` package was uninstalled in commit 56b2fe8 .
From https://github.com/kategengler/ember-feature-flags/blob/v6.0.0/README.md?plain=1#L7
> ### Note to users of `ember.js` >= 3.1
> Referencing the features service must be done using `get` as it is a proxy.
@MrChocolatine
Copy link
Member Author

PR initially created the 29 October 2023, after 2 weeks I am going to merge it soon.