Consider making MirageJS a peer dependency

[cah-brian-gantzler]

Currently MirageJS is a dependency.

With the recent changes that require users to import from MirageJS directly ie import { createServer } from 'miragejs';, I believe the applications package.json should include mirageJS in the devDependencies to indicate its direct use.

In my application I do have MirageJS in my package.json. MirageJS just release ^0.1.44. Updating my application package.json to ^0.1.44, I get the following error when building.

Build Error (WebpackBundler)

ember-rxid needs miragejs satisfying ^0.1.44, but we have version 0.1.43 because of ember-cli-mirage, ember-cli-mirage, ember-cli-mirage, ember-cli-mirage, ember-cli-mirage

I suggest we make MirageJS a peer dependency. Then make a blueprint that adds MirageJS to the applications package.json and/or update the installation documentation to instruct the consumer to also install MirageJS.

This will allow the consumer to specify which version of MirageJS it wishes to use similar to what ember-qunit has done.

[SergeAstapov]

this makes sense! Similar to what projects like https://github.com/adopted-ember-addons/ember-moment are doing

[SergeAstapov]

We can do similar thing, reference https://github.com/adopted-ember-addons/ember-moment/blob/master/src/index.js

[cah-brian-gantzler]

I will put together a PR.

[cah-brian-gantzler]

We can do similar thing, reference https://github.com/adopted-ember-addons/ember-moment/blob/master/src/index.js

What they are doing here is choosing between two different packages moment and moment-timezone. Not the version of one package.

What we want to do is more like this https://github.com/emberjs/ember-qunit/blob/master/package.json#L74 (moment does it as well https://github.com/adopted-ember-addons/ember-moment/blob/master/package.json#L105)

[SergeAstapov]

What they are doing here is choosing between two different packages moment and moment-timezone. Not the version of one package.

that's correct.

however my point is: on top of that (choose to use moment or moment-timzone), they mark moment as optional peer dependency (so that package manager does not install it automatically, see here) and throw an error if it's not found on this line https://github.com/adopted-ember-addons/ember-moment/blob/master/src/index.js#L27 and this error helps the app author to understand what went wrong and what needs to be done.

Def not something we have to do, but I prefer to completely delegate to the app the version of miragejs being used.
If you would open PR, we could also have a more targeted discussion about specifics there as I think this is great idea overall!

[cah-brian-gantzler]

As as I get a little time Ill do the PR

[cah-brian-gantzler]

After trying several variations, the reason the check here https://github.com/adopted-ember-addons/ember-moment/blob/master/src/index.js#L21 works is because everythere in the addon they use momentOrMomentTimezone to get the import ie https://github.com/adopted-ember-addons/ember-moment/blob/c80dd174c034d1ad62d2e016ea46d7de05d4cf98/src/helpers/moment-duration.js#L2

In order to do something similiar to this we would have to change everywhere we import from mirageJS to a wrapper function like momentOrMomentTimezone. I dont think thats reasonable.

If the user does not include mirageJS they should see something similiar to the following

ember-cli-mirage tried to import "miragejs" in "ember-cli-mirage/assert.js" but the package was not resolvable from /Users/brian.gantzler/source-personal/mirage-test/node_modules/ember-cli-mirage

If the docs say, you have to add miragejs to your package.json that should be enough for them to find the answer.

[cah-brian-gantzler]

Another interesting point is that if I do not include miragejs in 01-basic-app, the tests still work. This appears to be because miragejs is specified in the ember-cli-mirage-docs and the workspace resolves to it. Made even attempting some to tests what happens when not including miragejs from a test-package kinda difficult.

[NullVoxPopuli]

@cah-brian-gantzler I think the repo probably needs to migrate away from yarn@v1 before peers will behave correctly.
Yarn@v1 doesn't work, basically.

[cah-brian-gantzler]

Didnt realize that thanks.

Doesnt it really matter what tool is being used in the app that the is including this addon? as that is the one that will be using the package.json. It doesnt matter what this repo is using when its built does it? I think that would also apply to a V2 addon but not sure.

Tried to go to pnpm (on other projects) but was having difficulty with my work VPN.

[NullVoxPopuli]

it doesnt matter what this repo is using when its built does it?

mostly correct, but yarn@v1 kind of accidentally allows for a bunch of sloppiness which makes libraries using yarn not-as-good-of-a-steward-of-the-ecosystem. if, for example, pnpm were used with the settings mentioned here, you are nearly guaranteed maximum compatibility with all other package managers.

Doesnt it really matter what tool is being used in the app that the is including this addon?

the broader ecosystem, webpack, vite, etc require package.jsons to be correct, so in the way that libraries are required to correctly declare their dependencies, in matters how they're set up and managed, which is an extension, I suppose, of the package manager used to develop the project.

having difficulty with my work VPN

want to sync on discord?

[cah-brian-gantzler]

Would love to, pretty busy at work at the moment, can I get back with you when I have the time.