Clear mirage deprecations

[jrjohnson]

In v3.x functions from miragejs will no longer be exported by mirage and
must be pulled directly from miragejs (which I've added as a development dependency here).

The configuration system has been modified to match the upstream
expectations and by more explicit as well.

It seems like miragejs would probably need to get pushed out as a dependency or peedDependency in order to be imported from the addon directory, however I'm not sure what impact that would have on the build of the consuming app, how to document peerDependencies (as they aren't used much), or if there is a way around this.

I think the best path may be to remove Response and ship it as a local object which meets the interface needs of mirage, that's not usually a great idea though. Open to all suggestions!

[jelhan]

Thanks a lot for your contribution!

It seems like miragejs would probably need to get pushed out as a dependency or peedDependency in order to be imported from the addon directory, however I'm not sure what impact that would have on the build of the consuming app, how to document peerDependencies (as they aren't used much), or if there is a way around this.

I think the best path may be to remove Response and ship it as a local object which meets the interface needs of mirage, that's not usually a great idea though. Open to all suggestions!

An alternative would be to pull the Mirage support out in a separate package before landing this changes as discussed in #641. This would make it less risky to include miragejs as a dependency because we could assume that a consumer of that package is using Mirage. Doing so we could avoid the complexity of an optional peerDependency.

Using Embroider macros dependencySatisfies and importSync might be an alternative. Doing so should allow us to only import miragejs if consuming application has a dependency on that package. This is included as one example use case in the official documentation for Embroider macros: https://github.com/embroider-build/embroider/tree/main/packages/macros#dependencysatisfies But in my experience Embroider macros add quite some complexity. There is a reason why it hasn't reached a stable 1.0.0 release yet. This may have been improved as Embroider reached 1.0.0 3 days ago. 🎉

[jrjohnson]

Ok, I took a pass at doing this with macros, however due to embroider-build/embroider#199 this will fail unless both ember-cli-mirage and mirages are placed into peerDependencies. This code seems like a fairly ideal example of why that shouldn't be true, but I'm not sure how possible it is to update embroider to cover this situation. I wanted to push it as an example, but maybe pulling this into it's own package is the only way to move forward.

[jrjohnson]

I've added the optional peer dependencies and squashed commits, it looks like the embroider tests are hanging now though. Not sure if they need a restart or more attention. I'm going to run them locally and see what happens.

[gilest]

Not sure if they need a restart or more attention

Have restarted the CI for you 🤞🏻

[SergeAstapov]

Looks like something is still wrong with the import:

Built project successfully. Stored in "/tmp/tests-dist-2022024-1673-4hkmrc.cqznq".
82
not ok 1 Chrome 97.0 - [159 ms] - Integration | Helper | file-queue: filter is triggered when selecting files
83
    ---
84
        actual: >
85
            null
86
        stack: >
87
            Error: Could not find module `miragejs` imported from `(require)`
88
                at missingModule (http://localhost:7357/assets/vendor.js:256:11)
89
                at findModule (http://localhost:7357/assets/vendor.js:267:7)
90
                at requireModule (http://localhost:7357/assets/vendor.js:33:15)
91
                at Object.../../../externals/miragejs.js (http://localhost:7357/assets/chunk.c4aae3125db89d847f99.js:4207:18)
92
                at __webpack_require__ (http://localhost:7357/assets/chunk.a91ecf9989421fc5d8e6.js:1746:41)
93
                at upload (http://localhost:7357/assets/chunk.c4aae3125db89d847f99.js:851:157)
94
                at Server.routes (http://localhost:7357/assets/chunk.c4aae3125db89d847f99.js:3134:96)
95
                at Server.loadConfig (http://localhost:7357/assets/chunk.c433ad60b4345e672592.js:31746:14)
96
                at Server.config (http://localhost:7357/assets/chunk.c433ad60b4345e672592.js:31662:14)
97
                at new Server (http://localhost:7357/assets/chunk.c433ad60b4345e672592.js:31449:10)
98
        message: >
99
            beforeEach failed on filter is triggered when selecting files: Could not find module `miragejs` imported from `(require)`
100
        negative: >
101
            false
102
        browser log: |
103
    ...

[gilest]

I ran the embroider test locally to see what I can debug.

Quite interesting, ember-cli-mirage is included in require.entries but miragejs is not. Full entries dump here.

Presumably something to do with the way/format miragejs are configured – I'm not very familiar with these.

So (besides still using a deprecated re-export) it actually does work with:

const { Response } = importSync('ember-cli-mirage');

We could even move forward with that since it's deprecated until v3 (although not ideal).

Thoughts @SergeAstapov @jrjohnson

[SergeAstapov]

@gilest tried to debug on my local and can't see why embroider (or ember-auto-import) does not do the job properly.

Tried to use [email protected] with @embroider/[email protected] but no luck

[jelhan]

I think I have seen an issue, which sounds similar if sub-dependencies pull in older versions of Embroider. In that case it was fixed by pinning recent Embroider version using yarn resolutions feature. Maybe something you want to try out here as well.

[jrjohnson]

I just finished creating a simple as possible reproduction which also fails with a brand new v4 addon and just started a discord conversation. I'm at a loss.

We could even move forward with that since it's deprecated until v3 (although not ideal).

v3 is imminent. So hopefully a way forward can be found that is compatible.

[jrjohnson]

Well moving ember-auto-import to dependencies has fixed the embroider builds, however the normal build is now broken.

Annoyingly removing EAI from the resolutions fixes some, but breaks other scenarios. Hopefully there is an in between that will work.

[SergeAstapov]

Well moving ember-auto-import to dependencies has fixed the embroider builds

I think we stepped into grey zone of eai + @embroider/macros and not yet v2 addon.

I wonder if failure caused by any < v1.12 ember-auto-import@1 somewhere in the tree

[gilest]

This is painful ❤️ – what about moving it to dependencies and pinning it 😅

[SergeAstapov]

This is painful ❤️ – what about moving it to dependencies and pinning it 😅

this would be major version bump as consuming apps would have to migrate to e-a-i@2.

Alternative option is to add e-a-i@2 to dependencies only in embroider-safe and embroider-optimized scenarios to make ci green 🙈

[gilest]

this would be major version bump as consuming apps would have to migrate to e-a-i@2.

All current changes are being bundled into v5 until we can stand up a new docsite anyway.

Alternative option is to add e-a-i@2 to dependencies only in embroider-safe and embroider-optimized scenarios to make ci green 🙈

😅 wouldn't this cause problems for consumers using embroider?

[SergeAstapov]

😅 wouldn't this cause problems for consumers using embroider?

Not sure 100% tbh, I would check in real app to be confident, however high chances you are right.

[jrjohnson]

VICTORY 😤!!! In all my messing around I'd left EAI at 2.0 instead of ^2.0. Now that it's allowed to drift to 2.4 everything works as intended. I'm so happy I could scream.

[SergeAstapov]

VICTORY 😤!!! In all my messing around I'd left EAI at 2.0 instead of ^2.0. Now that it's allowed to drift to 2.4 everything works as intended. I'm so happy I could scream.

oh, so outdated version of EAI was the reason? can we now move it back to devDependencies if so?

[jrjohnson]

Unfortunately not @SergeAstapov, according to embroider-build/embroider#1091 it has to be in dependencies.

[SergeAstapov]

Unfortunately not @SergeAstapov, according to embroider-build/embroider#1091 it has to be in dependencies.

ahhhh, I see.
I thought when EAI listed in devDependencies it rolls in npm packages at build time but seem like there is limitation and it needs to be in dependencies.
Not totally clear why adding to devDependencies is not sufficient, but let it be that way

[gilest]

Thank you both for your help with this 😍