feat: update-ember-cli-project #213

rahulk94 · 2021-12-01T04:45:36Z

Description

This PR bumps this project up to the latest Ember CLI blueprint. This change has primarily been done using ember-cli-update followed up by commits to update tests and the source to follow newer lint rules.

This should be pretty straightforward / match other Ember CLI updates for addons as I've tried to do keep things as standard as possible here.

Not all lint rules have been addressed at this stage, I think upgrading the CLI version and later addressing lint rules is probably a more tactiful approach.

The tests are passing with Ember@3 LTS versions and failing with Ember@4 versions (which is fine since we're not aiming to add support for that in this PR as it drops lots of deprecated functionality). Embroider safe and optimized builds also fail atm and I'll do a separate PR to address that (as I think there's more issues around Babel with Embroider too).

New Feature

If this is a big feature with breaking changes, consider [opening an issue][issues] to discuss first. This is completely up to you, but please keep in mind that your PR might not be accepted.
Run unit tests to ensure all existing tests are still passing
NA - Add new passing unit tests to cover the code introduced by your PR
NA - Update the readme
Add some steps so we can test your cool new feature!
The PR title is in the conventional commit format: e.g. feat(<area>): added new way to do this cool thing #issue-number
Add your info to the contributors array in package.json!

Steps to Test

Running the tests and also the dummy app in this project should work as expected.

I've also integration tested this with our internal Ember app and verified this works as expected. Unfortunately I can't share that as it's closed source.

…ess it later

…n warn around ember-cli-babel

…oider & ember-cli-babel@7

commit-lint · 2021-12-01T04:45:40Z

Contributors

rahulk94

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR:

@Commit-Lint merge patch will merge dependabot PR on "patch" versions (X.X.Y - Y change)
@Commit-Lint merge minor will merge dependabot PR on "minor" versions (X.Y.Y - Y change)
@Commit-Lint merge major will merge dependabot PR on "major" versions (Y.Y.Y - Y change)
@Commit-Lint merge disable will desactivate merge dependabot PR
@Commit-Lint review will approve dependabot PR
@Commit-Lint stop review will stop approve dependabot PR

rahulk94 · 2021-12-01T04:46:48Z

addon/common/constants.js

+import { getOwnConfig } from '@embroider/macros';
+const APP_VERSION = getOwnConfig().version;


Ed F's fix from the Embroider support issue seems to have fixed this when using babel@7 deps
#210 (comment)

rahulk94 · 2021-12-01T04:49:36Z

addon/components/imgix-image-wrapped.js

-  init() {
-    this._super(...arguments);
-    deprecate(
-      'imgix-image-wrapped is deprecated and will be removed in ember-cli-imgix@3. Please migrate to using imgix-image instead.',


Please be aware that rather than updating this component I've just chopped it since it is probably best we cut a major now anyway (as the ember-cli-babel bump that comes with this PR probably warrants a major)

.travis.yml

rahulk94 · 2021-12-01T04:51:25Z

package.json

-    "ember-get-config": "0.2.4",
+    "ember-cli-babel": "^7.26.6",
+    "ember-cli-htmlbars": "^5.7.1",
+    "ember-get-config": "0.2.4 - 0.5.0",
    "ember-resize-aware": "1.2.0",
    "fastboot-transform": "^0.1.2",
    "imgix-core-js": "2.3.2",


In a separate PR we can switch this to use imgix/js-core since thats the new preferred name

Sounds good 👍

frederickfogerty

This looks good to me on the surface. I tried running the tests on CI (excluding lint), and it seems that the ember try cases are failing - even though the basic tests are succeeding. Take a look here for failing tests, and here for succeeding tests

.travis.yml

frederickfogerty · 2021-12-01T13:19:40Z

package.json

-    "ember-get-config": "0.2.4",
+    "ember-cli-babel": "^7.26.6",
+    "ember-cli-htmlbars": "^5.7.1",
+    "ember-get-config": "0.2.4 - 0.5.0",
    "ember-resize-aware": "1.2.0",
    "fastboot-transform": "^0.1.2",
    "imgix-core-js": "2.3.2",


Sounds good 👍

frederickfogerty · 2021-12-01T18:22:05Z

To be clear here, I'm pretty happy with the state of this PR. If we can get to a resolution about the Travis build and what to do to make sure it passes, then I'm happy to merge this.

rahulk94 · 2021-12-01T20:57:40Z

Hey @frederickfogerty thanks the quick first pass and getting the build to run on Travis.

Take a look here for failing tests

=== Scenario: ember-lts-3.20 ===================================================
Result: true

=== Scenario: ember-lts-3.24 ===================================================
Result: true

=== Scenario: ember-lts-3.28 ===================================================
Result: true

=== Scenario: ember-release ====================================================
To use these addons, your app needs ember-auto-import >= 2: ember-source
Result: false

=== Scenario: ember-beta =======================================================
To use these addons, your app needs ember-auto-import >= 2: ember-source
Result: false

=== Scenario: ember-canary =====================================================
To use these addons, your app needs ember-auto-import >= 2: ember-source
=================================================================================
Result: false

=== Scenario: ember-classic ====================================================
Result: true

=== Scenario: embroider-safe ===================================================
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received
The build has been terminated

Yeah so I'm not 100% sure what the best approach here is atm the moment. The Ember LTS series against version 3 all pass which is great, but the release, beta and canary are all failing due to us still using ember-auto-import (EAI) 1.x. EAI has recently had a major release which is not backwards compatible (I'll let you skim the upgrade/migration summary section rather than try re-explain it) and so that fails those builds. Ember release/beta/canary are all running Ember@4 as well which drops deprecated functionality (I have not checked if we are doing anything thats been dropped just yet). Embroider Ember Try builds are expected to fail at this stage (I haven't tried getting compatibility going and they just timeout weirdly).

Summing up, what are your thoughts on commenting the release, beta, canary, and Embroider options out in the Ember Try config and cutting a 3.0.0-beta.1 style release? I can then look at making Embroider options work since thats what I'm hear to try solve 😂 and do a 3.0.0-beta.2 and then we can see if we want/need to do anything else before cutting a 3.0.0 release?

Ember@4 support can be discussed in a later PR but depending on how easy it is/my capacity I may cheap out and just do a README note and Issue saying Ember we need to add Ember@4 compat soon.

frederickfogerty · 2021-12-02T12:18:44Z

Right, that makes sense as to why those builds are failing then, and I'm happy to comment out those stages and proceed as you suggested. It seems that the ember try environment variable (EMBER_TRY_SCENARIO) isn't working properly, I will need to fix this before merging as well. Do you have any ideas how to fix this? I could also just use one ember try since it seems that is already doing all of the steps 😅

Totally understand about Ember 4, and thanks for all your help until now!

rahulk94 · 2021-12-02T21:28:28Z

Hey @frederickfogerty I've removed the Ember@4 and Embroider stuff per our earlier discussion.

I've also updated the Travis.yml to call yarn test:ember-compatibility in the second stage so it should call Ember Try now, whereas earlier I think it was just doing ember test again 😅 . I reckon start the build and lets see how far we get now.

Oh also, commit lint is complaining at me but I can't tell what I've done wrong since it just links to generic docs rather than the broken rule so if you can tell let me know and I can touch it up 😂

…elaxed dependencies

rahulk94 · 2021-12-03T02:11:36Z

package.json

@@ -59,20 +84,21 @@
    "responsive"
  ],
  "dependencies": {
-    "babel-plugin-inline-package-json": "^2.0.0",
-    "babel-plugin-transform-object-rest-spread": "^6.26.0",


@frederickfogerty I've removed this plugin (and the newer equivalent) in the most recent commit so that consumers of this dependency don't specifically need the version we set. I was seeing some babel weirdness when integrating this with our app and realized its cause we'd declared a version new version of this as the minimum.

Fwiw its brought in transitively via ember-cli-babel so I reckon we're safe without it?

Sounds good to me 👍

sherwinski · 2021-12-03T19:17:07Z

Oh also, commit lint is complaining at me but I can't tell what I've done wrong since it just links to generic docs rather than the broken rule so if you can tell let me know and I can touch it up 😂

@rahulk94 Maybe I can chime in here. Commit lint expects the PR title to conform to a specific format, detailed here. So if you were to edit the PR title that check should pass.

rahulk94 · 2021-12-04T02:45:52Z

@sherwinski 🤦🏾‍♂️ cheers mate... Had a / instead of a : 🥲

frederickfogerty · 2021-12-06T15:59:57Z

Hey @rahulk94 - just leaving an interim comment here. I worked on Friday to try get the Travis build to pass, but didn't succeed. I will have a bit more time tomorrow, so hopefully I can have an update to you by then 👍 You can track my progress here for the branch "upgrade/update-ember-cli"

- use travis wait - build without embroider since currently not supported - use single travis step instead of one for each try step - add info about why builds are commented out - comment out lint task - remove branches from travis config

frederickfogerty

LGTM! Thanks so much for the help here @rahulk94 🙌

frederickfogerty · 2021-12-09T12:03:08Z

I got the tests passing here so I'll go ahead and merge this 🎉

frederickfogerty · 2021-12-09T12:17:34Z

@rahulk94 I opened up this PR to release v3 of this library. If you have any improvements on the release notes there, I'm all ears. Thanks!

Rahul Kumar added 30 commits November 23, 2021 18:00

v3.1.2...v3.28.4

630fa3b

Update Yarn.lock file post update

fae81ae

Apply ember-angle-brackets-codemod

c53526a

Drop Ember 2.16 from Ember try matrix

6dbd9c3

Manually fix up curly component invocation lint errors

a416aef

Add lint script for just JS

1d9a4ec

Add missing eslint plugins post ember-cli-update

0db51d5

Run Yarn lint:fix to fix all autofixable issues

b223ead

Apply ember-test-helpers-codemod

03b54e2

Manually fix up qunit/require-expect in imgix-image-test

2f79a85

Fix up ember/new-module-imports lint rules

8343bbf

Fixup ember/no-classic-classes rule on router

2c3ca5d

Fixup double export in dummy router

feb6e43

Explicitly declare ember-source-channel-url for Ember Try config

df5dec5

Remove explicit babel plugin declarations

53e7452

Fixup ember/no-unnecessary-route-path-option lint rule

29a22ad

Fixup ember/no-try-invoke rule

5bb40c5

Update contribution guide to refer to Yarn

8fa0ce3

Remove tests/.eslintrc as its all managed through the parent config now

3e016ea

Fix ember/no-get lint rule

8dde9b8

Autofix require-computed-macros lint rule

2f29ec3

Set ember/no-jquery lint rule to warn as we upgrade Ember CLI to addr…

de6b46d

…ess it later

Fix no-unused-vars and no-redeclare rules in tests for globals

aa0f2d7

Bring in @babel/plugin-proposal-object-rest-spread plugin

a2aad19

Fix JS syntax errors causing build failures (woops)

b557dfd

Update browserlists for caniuse-lite to more recent browsers

4ba060c

Run npx yarn-deduplicate --packages ember-cli-babel to fix deprecatio…

78a312c

…n warn around ember-cli-babel

Lookup app version using Embroider macros for compatibility with embr…

e9a3802

…oider & ember-cli-babel@7

Fix up imgix-image-test test cases and remove jquery usage

d0153a5

Fix up no implicit this rule for imgix-image-wrapped template

f1e6bba

rahulk94 commented Dec 1, 2021

View reviewed changes

.travis.yml Show resolved Hide resolved

rahulk94 commented Dec 1, 2021

View reviewed changes

frederickfogerty reviewed Dec 1, 2021

View reviewed changes

Remove linting from test script since that'll fail in CI currently

48d503d

Rahul Kumar added 5 commits December 3, 2021 10:22

Remove release, beta and canary builds from Ember Try config for now

8514c66

Remove Embroider compatibility builds from Ember Try config

65b680c

Woops forgot to update the Travis config

2db333c

Remove scenario which is not in Ember Try config from Travis env vars

152f4b2

Use test compatibility script in build on floating dep stage

1023c9e

rahulk94 requested a review from frederickfogerty December 2, 2021 23:27

Remove explicit babel rest spread plugin so consumers can have more r…

882e0f3

…elaxed dependencies

rahulk94 commented Dec 3, 2021

View reviewed changes

rahulk94 changed the title ~~feat/update-ember-cli-project~~ feat:update-ember-cli-project Dec 4, 2021

rahulk94 changed the title ~~feat:update-ember-cli-project~~ feat: update-ember-cli-project Dec 4, 2021

frederickfogerty added 2 commits December 9, 2021 13:01

chore: fix indentation in README

becce5a

chore: update traivs config

982b022

- use travis wait - build without embroider since currently not supported - use single travis step instead of one for each try step - add info about why builds are commented out - comment out lint task - remove branches from travis config

frederickfogerty approved these changes Dec 9, 2021

View reviewed changes

frederickfogerty merged commit f7bd774 into imgix:main Dec 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update-ember-cli-project #213

feat: update-ember-cli-project #213

rahulk94 commented Dec 1, 2021 •

edited

Loading

commit-lint bot commented Dec 1, 2021

rahulk94 Dec 1, 2021

rahulk94 Dec 1, 2021

rahulk94 Dec 1, 2021

frederickfogerty Dec 1, 2021

frederickfogerty left a comment

frederickfogerty Dec 1, 2021

frederickfogerty commented Dec 1, 2021

rahulk94 commented Dec 1, 2021

frederickfogerty commented Dec 2, 2021

rahulk94 commented Dec 2, 2021 •

edited

Loading

rahulk94 Dec 3, 2021

frederickfogerty Dec 3, 2021

sherwinski commented Dec 3, 2021 •

edited

Loading

rahulk94 commented Dec 4, 2021

frederickfogerty commented Dec 6, 2021

frederickfogerty left a comment

frederickfogerty commented Dec 9, 2021

frederickfogerty commented Dec 9, 2021

		import { getOwnConfig } from '@embroider/macros';
		const APP_VERSION = getOwnConfig().version;

feat: update-ember-cli-project #213

feat: update-ember-cli-project #213

Conversation

rahulk94 commented Dec 1, 2021 • edited Loading

Description

New Feature

Steps to Test

commit-lint bot commented Dec 1, 2021

Contributors

rahulk94 Dec 1, 2021

Choose a reason for hiding this comment

rahulk94 Dec 1, 2021

Choose a reason for hiding this comment

rahulk94 Dec 1, 2021

Choose a reason for hiding this comment

frederickfogerty Dec 1, 2021

Choose a reason for hiding this comment

frederickfogerty left a comment

Choose a reason for hiding this comment

frederickfogerty Dec 1, 2021

Choose a reason for hiding this comment

frederickfogerty commented Dec 1, 2021

rahulk94 commented Dec 1, 2021

frederickfogerty commented Dec 2, 2021

rahulk94 commented Dec 2, 2021 • edited Loading

rahulk94 Dec 3, 2021

Choose a reason for hiding this comment

frederickfogerty Dec 3, 2021

Choose a reason for hiding this comment

sherwinski commented Dec 3, 2021 • edited Loading

rahulk94 commented Dec 4, 2021

frederickfogerty commented Dec 6, 2021

frederickfogerty left a comment

Choose a reason for hiding this comment

frederickfogerty commented Dec 9, 2021

frederickfogerty commented Dec 9, 2021

rahulk94 commented Dec 1, 2021 •

edited

Loading

rahulk94 commented Dec 2, 2021 •

edited

Loading

sherwinski commented Dec 3, 2021 •

edited

Loading