fix ci #379

NullVoxPopuli · 2022-02-15T16:45:17Z

Most of the diff is from the yarn.lock file

The commits show a progression of why or what changed if you like reviewing that way. I'm also happy to squash if that's what folks would like.

Summary

changes supported ember version to 3.12, 3.16 to 3.28, 3.24
- (canary, beta, release, default) still present
updates @glimmer/component's dependencies to be compatible with the newer ember versions
- mainly: ember-auto-import@v2 is required for ember 4+
applied addon blueprint over @glimmer/component (for the specified ember-source in that package.json) -- most of the diff is from these sorts of changes
upgrades @glimmer/vm (and related dependencies)
ember-component-manager changed from using Ember global apis to @ember/destroyable
- I've been under the impression that this is the correct way to handle destruction
ember-qunit and related changes are part of the v5 upgrade
html-inlen-precomile premoved in favor of ember-cli-htmlbars
minimum typescript version changed from 3.1 to 4.0
https://github.com/glimmerjs/glimmer.js/pull/379/files#diff-f17a36b7758421e673e4dae6c85e48d2a576da5bf873a4582ea563bcd2e5d742

Points of Interest

modifiers test bumped capabilities: https://github.com/glimmerjs/glimmer.js/pull/379/files#diff-167ae90f1eff8de3995ae4a358d0719e15e69a218958bb2c20d67a4217ac4125
lookupPartial removed: https://github.com/glimmerjs/glimmer.js/pull/379/files#diff-61c001d9dd145b884ddcc7008409f0a351989c201ba14647457d03490ffd0e1f
change in destruction https://github.com/glimmerjs/glimmer.js/pull/379/files#diff-0d4d98f5acbcd1563d48f203d36e82c39815d1a0c7eea0321fba0bfae3b81483

What this PR Unblocks

Adds @cached decorator #358

locks · 2022-02-15T17:46:24Z

Would this close #230 ?

NullVoxPopuli · 2022-02-15T17:55:06Z

partially -- I think deciding on a policy should still happen. I'm in favor of mirroring ember's support here

…ry-scenarios

…ber-cli/ember-cli-lodash-subset#39

chancancode · 2022-02-24T00:45:32Z

.github/workflows/ci.yml

@@ -51,7 +51,7 @@ jobs:

    strategy:
      matrix:
-        ember-version: [default, release, beta, canary, lts-3.12, lts-3.16]
+        ember-version: [default, release, beta, canary, lts-3.28, lts-3.24]


Was dropping support tied to the requirements for the @cached decorator, or is it just because/part of the blueprint update? (i.e. are we able to keep the older Ember versions if we wanted to?)

I can give it a go -- I was mainly going with the ember support policy, cause I don't know what the glimmer one is

i guess it's fine 🙃
(assuming we trust the tests)

Sorry, I wasn't implying that we should do it one way or the other, just wanted to know if it was deliberate/required or out of convince (what could be fine). Although, now that you proved that it works (at least according to the tests), if it doesn't end up hurting anything, I personally would be inclined to keep the support to aid adoption (though we should also include 3.20 in the matrix in that case).

However, I do think it is important to double check and make sure we in fact could support the @cached decorator on all of these versions, at least before we release 2.0 final.

chancancode · 2022-02-24T05:02:32Z

packages/@glimmer/component/tests/dummy/config/optional-features.json

@@ -0,0 +1,5 @@
+{
+  "application-template-wrapper": false,
+  "jquery-integration": false,


This doesn't have the async observer stuff, intentional?

It turns out that I had an extra optional-features file.

This file, the traditional location, is the only place where the test runner looks for these options, afaict. I've moved the more complete optional-features.json to this location so that it's proper octane (including async observers)

chancancode · 2022-02-24T05:04:21Z

test/types/index.d.ts

@@ -1 +1 @@
-// Minimum TypeScript Version: 3.1
+// Minimum TypeScript Version: 4.0


Normally I think there are some consequences with this, but given that we apparently need to release a major version bump anyway, this is probably fine. Though @chriskrycho should probably confirm just in case. (I am not sure how much of the @glimmer/* types ended up being consumed directly, as opposed to being replaced/superseded by the Glint wrappers anyway.)

This is indeed breaking! And at this point the majority of the ecosystem is not using Glint, in past because addons cannot do so. (That’s one of the big motivations for moving the Signature RFC forward.) If this is intending to target a 2.0 release, the breaking change is fine. Otherwise we should leave it at its previous value.

We already have to do a breaking change anyway without doing an enormous (imo) amount of work to support from ember 3.12 to current (macros, no more partials, etc)

That seems fine, then, with two caveats:

I believe there are a number of other changes already queued up for a v2, so we need to evaluate those as part of this work.

We need to make sure that end users can actually successfully have both v1 and v2 active in an app's codebase, since it will take some time to migrate the ecosystem.

Two additional considerations:

We can support the Glint Signature changes without a breaking change, but it would be much easier to not. We need to decide on what to do with that.

There are other changes we want to make to align the Glimmer.js API with the version of the Glimmer component API available in Ember.js, specifically around lifecycle hooks. I am not sure if those have yet landed, but we need to follow up and evaluate those before we actually publish the v2.

what specific things do we need to do to move forward?
(I'm not clear on what next steps are)

For the most part, I don't think should affect this PR, considering we already released 17 (!) betas for 2.0.0. But those are probably checklist items for the 2.0.0 final.

chancancode · 2022-02-24T05:10:31Z

ember-component-manager changed from using Ember global apis to @ember/destroyable

I've been under the impression that this is the correct way to handle destruction

Hmm, actually this might be a good enough reason to drop < 3.24, or else we probably need to do some Embroider macro things to make it work. But in any case, I am confused why the tests are passing?

chancancode · 2022-02-24T05:11:18Z

Thanks for putting this together and providing the context for the changes, they are super helpful!

…tually work, considering the destroyable apis weren't added until 3.22

NullVoxPopuli · 2022-02-24T14:39:02Z

enough reason to drop < 3.24

I've re-dropped 3.12 and 3.16. I think the destroyable stuff was added in 3.22, and I don't trust the test suite with ember-source < 3.22

chancancode · 2022-02-24T21:27:59Z

Seems like we will need a checklist for the 2.0 release outside of this PR (and one of the items should be landing the @cached decorator), @chriskrycho do you mind making one (or make a milestone, whatever)? Seems like you have more context on what is needed.

Separately, I am pretty concerned that the test were passing with 3.12/3.16 in the matrix since there is no destroyable APIs. @NullVoxPopuli can you investigate what's up, or at least open an issue? Are we just missing some test coverage around destruction, or were the tests running at all?

In any case both of those things are not directly related to this PR, I think we are good to proceed. Also wouldn't mind if @rwjblue could give this a look when he is back to see if I missed anything around the upgrades. Thanks again @NullVoxPopuli for taking care of this!

NullVoxPopuli · 2022-02-24T22:01:28Z

Thanks! <3

Issue here (for now, I gotta pivot to some embroider work for the near near term)
#382

chriskrycho · 2022-02-24T23:16:39Z

Thanks for suggesting that, @chancancode – I just opened #383 to that end!

NullVoxPopuli added 3 commits February 15, 2022 12:55

Drop support for Ember < 3.24

9f9e796

Add webpack and ember-auto-import@2 to glimmer/component and update t…

4ad7555

…ry-scenarios

Upgrade some dependencies in @glimmer/component used for testing

5aae2cc

NullVoxPopuli force-pushed the fix-ci-34f8a7e7ps branch from fa1a606 to 5aae2cc Compare February 15, 2022 17:56

NullVoxPopuli added 15 commits February 15, 2022 14:06

Update testem configs

3b872ab

Sync dependencies with the 3.24 blueprint

6e1db8e

Upgrade node / volta config

0d3d7fb

Lint fix

4453191

types pass now

1ecf676

Add optional features

b7f8737

Add optional features

53ff239

Update babel fixtures

1b2d56e

Scope isn't present when not set

137dd4f

Upgrade qunit setup for @glimmer/component

25d5384

More modernization

d519bfc

Add embroider/test-setup

a5010ac

Migrate to @ember/destroyable and avoid the Ember global

5b0c54f

Re-roll lockfile

12b2a76

Upgrade ember-cli

2fcd697

NullVoxPopuli mentioned this pull request Feb 16, 2022

Cannot add property volatileFlag, object is not extensible ember-cli/ember-cli-lodash-subset#39

Open

Ember-cli 3.27 is the max version I can use without running in to: em…

e757dc0

…ber-cli/ember-cli-lodash-subset#39

lifeart approved these changes Feb 17, 2022

View reviewed changes

chancancode reviewed Feb 24, 2022

View reviewed changes

Re-add ember 3.12 and 3.16 to glimmer/component try-scenarios

6e3ba54

chancancode reviewed Feb 24, 2022

View reviewed changes

NullVoxPopuli added 2 commits February 24, 2022 09:07

Re-remove support for 3.16 and 3.12 because there is *no way* they ac…

fe0d39d

…tually work, considering the destroyable apis weren't added until 3.22

Place optional-features in the place where ember test expects them to be

60b5237

chancancode approved these changes Feb 24, 2022

View reviewed changes

chancancode merged commit ee9fbf0 into glimmerjs:master Feb 24, 2022

NullVoxPopuli mentioned this pull request Feb 24, 2022

Todo: add tests around component destruction #382

Open

NullVoxPopuli deleted the fix-ci-34f8a7e7ps branch February 25, 2022 00:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix ci #379

fix ci #379

NullVoxPopuli commented Feb 15, 2022 •

edited

locks commented Feb 15, 2022

NullVoxPopuli commented Feb 15, 2022

chancancode Feb 24, 2022

NullVoxPopuli Feb 24, 2022

NullVoxPopuli Feb 24, 2022

chancancode Feb 24, 2022

chancancode Feb 24, 2022

NullVoxPopuli Feb 24, 2022

chancancode Feb 24, 2022

chriskrycho Feb 24, 2022

NullVoxPopuli Feb 24, 2022 •

edited

chriskrycho Feb 24, 2022

NullVoxPopuli Feb 24, 2022

chancancode Feb 24, 2022

chancancode commented Feb 24, 2022

chancancode commented Feb 24, 2022

NullVoxPopuli commented Feb 24, 2022

chancancode commented Feb 24, 2022

NullVoxPopuli commented Feb 24, 2022

chriskrycho commented Feb 24, 2022

		@@ -1 +1 @@
		// Minimum TypeScript Version: 3.1
		// Minimum TypeScript Version: 4.0

fix ci #379

fix ci #379

Conversation

NullVoxPopuli commented Feb 15, 2022 • edited

locks commented Feb 15, 2022

NullVoxPopuli commented Feb 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli Feb 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chancancode commented Feb 24, 2022

chancancode commented Feb 24, 2022

NullVoxPopuli commented Feb 24, 2022

chancancode commented Feb 24, 2022

NullVoxPopuli commented Feb 24, 2022

chriskrycho commented Feb 24, 2022

NullVoxPopuli commented Feb 15, 2022 •

edited

NullVoxPopuli Feb 24, 2022 •

edited