add default overrides for generated beta and canary configs #191
Conversation
|
Won't custom scenarios also need this override? I was chatting the other day in discord about this. I think it is possible that ember-try should automatically add overrides/resolutions for some (if not all) packages defined in a scenario. |
|
I'm not too sure about the overrides for everything 🤔 I feel like the whole point of ember-try is to try ember-source on different versions so it makes sense that ember-source has these overrides My personal plan is that I was going to get ember-try released with support for overrides and then I was going to PR this override to the default blueprint too. I think that is probably a good start and then we can worry about overriding everything later. What do you think? |
|
I thought that for quite awhile but I have been convinced that ember-try gets used beyond that use case. ember-try can be used for any npm package for which you want to test against multiple versions. But ignoring that, even considering just the ember-source use case, sometimes you need to change other packages to "go with" that particular ember-source version. Most recently, we had to specify ember-resolver for certain scenarios. On the version of npm you are using, wouldn't that also need an override to be happy? |
| @@ -33,6 +33,9 @@ async function generateScenariosFromSemver(semverStatements, availableVersions) | |||
| devDependencies: { | |||
| 'ember-source': versionNum, | |||
| }, | |||
| overrides: { | |||
There was a problem hiding this comment.
These configs are used whether npm, yarn, or now pnpm is used. What does overrides do in pnpm? Does pnpm also support the $ember-source syntax?
Is it ignored properly in yarn? Does a new project in yarn have the same problem and need resolutions?
There was a problem hiding this comment.
so [email protected] never did strict peer checks
pnpm only listens to overrides that are inside a pnpm keyed object, and pnpm doesn't have the exact same issue because pnpm considers pre-releases as matching when their ranges match
There was a problem hiding this comment.
I wanted to confirm that both pnpm and yarn will ignore overrides, if so, I think this approach is fine for the automated config, but if we put it in the blueprint I think it will be confusing to non-npm users, so it should be conditional and only there for npm users.
There was a problem hiding this comment.
Will this hide peer dependency issues? For example, in the run up to 5.0, ember-try runs for addons caught that various other addons had incorrect (or overly restrictive) peer dependency on ember-source and needed to be loosened.
There was a problem hiding this comment.
this will only peer dependency issues with ember-source and only on auto-generated configs. When I say that I plan to add this to the default blueprint I mean only for release, canary, and beta because they will never work because of npm considering the pre-releases not inside legit ranges 😫
There was a problem hiding this comment.
beta and canary I understand, release should work though
mansona
force-pushed
the
default-overrides
branch
from
4529510
to
0f01552
Compare
mansona
changed the title
I'm in the process of getting ember-cli/ember-try#920 to pass and I came across a curious problem. All of the auto-generated scenarios all need to have an override to make npm happy 😫
This PR adds that override and makes sure that tests pass 👍