Consider making MirageJS a peer dependency about ember-cli-mirage HOT 12 OPEN

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

from ember-cli-mirage.

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

from ember-cli-mirage.

I will put together a PR.

from ember-cli-mirage.

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)

from ember-cli-mirage.

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!

from ember-cli-mirage.

As as I get a little time Ill do the PR

from ember-cli-mirage.

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.

from ember-cli-mirage.

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.

from ember-cli-mirage.

@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.

from ember-cli-mirage.

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.

from ember-cli-mirage.

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?

from ember-cli-mirage.

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

from ember-cli-mirage.