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

Downgrade ember-cli if ember-source <= 3.28 is gonna be used with ember-cli 5+ #192

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lolmaus
Copy link

@lolmaus lolmaus commented Aug 10, 2023

This is continuation of #191 .

It's not a complete solution. Please consider it to be a first iteration.

@kategengler
Copy link
Member

👋 I think there may be some confusion about when this package is used; it is only used when packages use versionCompatibility feature of ember-try https://github.com/ember-cli/ember-try/blob/c92dc695b1424f8db9a041ddaf2e1647012d5948/lib/utils/config.js#L48. Most users of ember-try do not use this and use the scenarios generated in the blueprints, so to get this is behavior, they would need to put it as part of their scenarios.

As an aside, v3.28 is no longer supported by Ember itself and I would personally recommend addon authors drop support.

@mansona
Copy link
Member

mansona commented Aug 14, 2023

Hey @kategengler sorry I should have given you a heads-up on this, I'm working with @lolmaus on this and it's related to the work that we discussed regarding having ember-try-config be a "source of truth" of valid versions of ember-source and other parts of the blueprint. This would essentially replace most of what I was trying to achieve in #191

I understand that this is targeting 3.28's incompatibility with ember-cli and that is quite an old version (and not supported any more) but this PR is only the first of many that we're going to open that will add different "rules" to the config as necessary 👍

@lolmaus
Copy link
Author

lolmaus commented Aug 14, 2023

As an aside, v3.28 is no longer supported by Ember itself and I would personally recommend addon authors drop support.

Regarding the support of 3.28.

An addon may be compatible with Ember 3.28, but a scenario targeting 3.28 is invalid by default, due to subdependency mismatch.

I believe it's a huge bummer for all addons to claim incompatibility with a certain Ember version because of this reason alone, while they are compatible otherwise.

On top of that, Embroider does support 3.28 in its test suite as an LTS.

@kategengler
Copy link
Member

The comment about 3.28 was an aside because I personally believe our community spends too much effort on supporting very old versions, but my main point about this package only being used for versionCompatibility stands.

We had briefly discussed this wanting to be a source of truth but I think I also mentioned it would be preferable to be more composable. I think it would be interesting if this package contained scenarios similar to the embroider scenarios that can be imported in the config and used or extended there. In that case, the config could do:

const { Scenarios } = require('ember-try-config);

module.exports = async function () {
  return {
    scenarios: [
      Scenarios['canary'](),
      Scenarios['beta']({
         allowedToFail: false,  
      }),
      Scenarios['3.28'](), 
      Scenarios['5.2']({
          devDependencies: {
             'my-custom-dep': '12.0.9',
          }
       })           
   ]
  }
}

The passed in object would be deep-merged with the default version of that scenario.

Some questions:

  • What do we do about packages that are not required? (optional addons that need to be particular version for a particular version of Ember such as ember-data)
  • Aside: should be convert the try config to mjs?

@mansona
Copy link
Member

mansona commented Aug 15, 2023

So first of all, i very much love your suggestion that this could provide the basis for "default versions" in an addon's actually ember-try config. This solves a huge problem in the community, and something that came up multiple times when I was pairing with @lolmaus about this: "How does this change actually fix the scenarios for ".

I would like to separate that idea though as a "follow on" that we work on, because the main thrust of this PR from my perspective is to unblock ember-cli/ember-try#920 which needs this PR and the subsequent PRs that will be adding more rules for different packages (ember-qunit, ember-resolver, test-helpers, etc.) and I wouldn't want to block that effort on the design of this being Just Right™️

To answer your question What do we do about packages that are not required: we have the ability to inspect what packages are being used. You can see that in this PR we are inspecting the project's dependencies to determine if the version of ember-cli is "too high" to work for ember-source 3.28. It turns out that where this package is being used in ember-try we were already passing in the project so this will work out of the box as it is. If we need to do the same sort of introspection for things like ember-data we can 👍

@kategengler
Copy link
Member

I think it would make a lot more sense to unblock ember-cli/ember-try#920 the way an existing project would currently have to: by either changing the versions used for tests that use ember-try-config or by adding to an ember-try config to change the ember-cli version (same named scenarios get merged with the ember-try-config generated config) to change the ember-cli version.

There are not many uses of versionCompatibility in the wild. I spot checked several addons that have it in their package.json but they also have ember-try config files straight out of the blueprint.

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.

3 participants