miragejs / ember-cli-mirage

An Ember Addon to easily add Mirage JS to your Ember app.
http://ember-cli-mirage.com
MIT License
864 stars 440 forks source link

@embroider/macros causing builds to fail > v2.2 #2343

Closed pjcarly closed 9 months ago

pjcarly commented 2 years ago

After upgrading our dependencies builds started crashing with the below error messages:

Build Error (broccoli-persistent-filter:Babel > [Babel: @embroider/macros]) in @embroider/macros/runtime.js

<snip>/@embroider/macros/runtime.js: The "path" argument must be of type string. Received an instance of Object

and

Build Error (broccoli-persistent-filter:Babel > [Babel: @embroider/macros]) in @embroider/macros/runtime.js

<snip>/@embroider/macros/runtime.js: @babel/template placeholder "1000": Expected string substitution

Our investigation lead us to the bump from mirage version 2.2 to 2.4, by locking it to 2.2 builds weren't crashing anymore (2.3 is confirmed to also crash).

I tried reproducing the error in a standalone app but couldn't get it to crash. My guess it has something to do with dependencies from different addons on different versions of the same package (I noticed ember-power-select also has a dependency on @embroider/macros in our app, but that was not causing the issue).

Nevertheless, the issue stands, perhaps someone can find this with similar problems, and we can narrow it down even further.

SergeAstapov commented 2 years ago

I tried reproducing the error in a standalone app but couldn't get it to crash.

I guess this resurface some issues that exist in older @embroider/macros version ember-cli-mirage > v2.2 is using.

The problem with bumping @embroider/macros is that it requires Node.js 12+ and ember-cli-mirage v2 supports Node.js 10. Upgrading @embroider/macros to 1.0.0 would be a breaking change as it would ultimately require Node.js 12+.

I would to try ember-cli-mirage@v3.0.0-alpha.1 as it upgrades @embroider/macros and should be pretty stable (although you should update all the import paths remove in #2314).

Another option is to use resolutions and enforce @embroider/macros@^1.0.0.

I'm hesitant to bump @embroider/macros to v1 in ember-cli-mirage as it's breaking change.

Another option to move forward is to release stable v3 with @embroider/macros@1.0.0 and rollback #2297 in v2 (which is not great option as well IMO).

Ultimately and unfortunately, issues is not in ember-cli-mirage but in older version of @embroider/macros used in v2.

pjcarly commented 2 years ago

I'm up for bumping to v3.0.0-alpha.1, but how do import paths need to be updated? Currently having imports like this:

import { Request, Response } from "miragejs";
import { Server } from "miragejs/server";

Cheers for the response.

cah-brian-gantzler commented 2 years ago

We are also talking about the imports in your mirage models, adapters, etc

For example: many people have

import { Model, belongsTo } from 'ember-cli-mirage';

which should be changed to

import { Model, belongsTo } from 'miragejs';

Anything that was extracted to MirageJS (almost everything) should be imported frommiragejs

The things left that ember-cli-mirage exports is setupMirage and things used in defining the server as below.

Previously your routes were the only thing defined in the config.js, now your config.js should look like this

import {
  discoverEmberDataModels,
} from 'ember-cli-mirage';
import { createServer } from 'miragejs';

export default function (config) {
  let finalConfig = {
    ...config,
    models: { ...discoverEmberDataModels(), ...config.models },
    routes,
  };

  return createServer(finalConfig);
}

function routes() {
// define your routes here
}

This now makes our definition of a server look like MirageJS documentation.

cah-brian-gantzler commented 2 years ago

v3.0.0-alpha.1 still supports the routes only config, but deprecated, will be releasing an alpha.2 that removes that soon

pjcarly commented 2 years ago

v3 isn't working for me. Can't even get ember-cli to build.

Build Error (broccoli-persistent-filter:Babel > [Babel: @embroider/macros]) in @embroider/macros/runtime.js

<snip>/@embroider/macros/runtime.js: @babel/template placeholder "APP": Expected string substitution

stack:

(node:349084) UnhandledPromiseRejectionWarning: Error: @babel/template placeholder "APP": Expected string substitution
    at applyReplacement (<snip>/node_modules/@babel/template/lib/populate.js:74:13)
    at <snip>/node_modules/@babel/template/lib/populate.js:44:7
    at Array.forEach (<anonymous>)
    at populatePlaceholders (<snip>/node_modules/@babel/template/lib/populate.js:42:43)
    at <snip>/node_modules/@babel/template/lib/string.js:20:51
    at <snip>/node_modules/@babel/template/lib/builder.js:75:14
    at Object.buildLiterals (<snip>/node_modules/ember-element-helper/node_modules/@embroider/macros/src/babel/evaluate-json.js:377:73)
    at Object.inlineRuntimeConfig (<snip>/node_modules/ember-element-helper/node_modules/@embroider/macros/src/babel/get-config.js:107:55)
    at PluginPass.enter (<snip>/node_modules/ember-element-helper/node_modules/@embroider/macros/src/babel/macros-babel-plugin.js:77:38)
    at newFn (<snip>/node_modules/@babel/traverse/lib/visitors.js:177:21)
    =============
    at Object.buildLiterals (<snip>/node_modules/ember-element-helper/node_modules/@embroider/macros/src/babel/evaluate-json.js:377:34)
    at Object.inlineRuntimeConfig (<snip>/node_modules/ember-element-helper/node_modules/@embroider/macros/src/babel/get-config.js:107:55)
    at PluginPass.enter (<snip>/node_modules/ember-element-helper/node_modules/@embroider/macros/src/babel/macros-babel-plugin.js:77:38)
    at newFn (<snip>/node_modules/@babel/traverse/lib/visitors.js:177:21)
    at NodePath._call (<snip>/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (<snip>/node_modules/@babel/traverse/lib/path/context.js:40:17)
    at NodePath.visit (<snip>/node_modules/@babel/traverse/lib/path/context.js:100:31)
    at TraversalContext.visitQueue (<snip>/node_modules/@babel/traverse/lib/context.js:103:16)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:349084) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:349084) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
cah-brian-gantzler commented 2 years ago

Interestingly I get that same error when I try to update ember font-awesome and staying on ember-cli-mirage 2.4. I had thought it was a font-awesome problems.

Could be another embroider migration problem.

If your app using auto-import V1 or V2?

Feel a lot of addons are probably encountering these mirgration issues

From: PJ Carly @.> Date: Thursday, February 3, 2022 at 6:32 AM To: miragejs/ember-cli-mirage @.> Cc: Gantzler, Brian @.>, Comment @.> Subject: Re: [miragejs/ember-cli-mirage] @embroider/macros causing builds to fail > v2.2 (Issue #2343)

    External Email – Please use caution before opening attachments or clicking links

v3 isn't working for me. Can't even get ember-cli to build.

Build Error (broccoli-persistent-filter:Babel > [Babel: @embroider/macros]) in @embroider/macros/runtime.js

@.***/macros/runtime.js: @babel/template placeholder "APP": Expected string substitution

stack:

(node:349084) UnhandledPromiseRejectionWarning: Error: @babel/template placeholder "APP": Expected string substitution

at applyReplacement ***@***.***/template/lib/populate.js:74:13)

at ***@***.***/template/lib/populate.js:44:7

at Array.forEach (<anonymous>)

at populatePlaceholders ***@***.***/template/lib/populate.js:42:43)

at ***@***.***/template/lib/string.js:20:51

at ***@***.***/template/lib/builder.js:75:14

at Object.buildLiterals ***@***.***/macros/src/babel/evaluate-json.js:377:73)

at Object.inlineRuntimeConfig ***@***.***/macros/src/babel/get-config.js:107:55)

at PluginPass.enter ***@***.***/macros/src/babel/macros-babel-plugin.js:77:38)

at newFn ***@***.***/traverse/lib/visitors.js:177:21)

=============

at Object.buildLiterals ***@***.***/macros/src/babel/evaluate-json.js:377:34)

at Object.inlineRuntimeConfig ***@***.***/macros/src/babel/get-config.js:107:55)

at PluginPass.enter ***@***.***/macros/src/babel/macros-babel-plugin.js:77:38)

at newFn ***@***.***/traverse/lib/visitors.js:177:21)

at NodePath._call ***@***.***/traverse/lib/path/context.js:53:20)

at NodePath.call ***@***.***/traverse/lib/path/context.js:40:17)

at NodePath.visit ***@***.***/traverse/lib/path/context.js:100:31)

at TraversalContext.visitQueue ***@***.***/traverse/lib/context.js:103:16)

(Use node --trace-warnings ... to show where the warning was created)

(node:349084) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)

(node:349084) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

— Reply to this email directly, view it on GitHubhttps://github.com/miragejs/ember-cli-mirage/issues/2343#issuecomment-1028899450, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AALVLRQ5VO72MNN4AARZCK3UZJRUTANCNFSM5M2XSMRQ. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you commented.Message ID: @.***>


This message is for the designated recipient only and may contain privileged, proprietary or otherwise private information. If you have received it in error, please notify the sender immediately and delete the original. Any other use of the email by you is prohibited.

Dansk - Deutsch - Espanol - Francais - Italiano - Japanese - Nederlands - Norsk - Portuguese - Chinese Svenska: http://www.cardinalhealth.com/en/support/terms-and-conditions-english.html

pjcarly commented 2 years ago

using ember-auto-import v2 in my dependencies however there is still a v1 present in my lock file (from ember-intl).

SergeAstapov commented 2 years ago

using ember-auto-import v2 in my dependencies however there is still a v1 present in my lock file (from ember-intl).

@pjcarly could you please check if it's EAI 1.12+ as it has @embroider/* in dependencies.

Regarding /@embroider/macros/runtime.js: @babel/template placeholder "APP": Expected string substitution

this issue happens when you have outdated @embroider/* packages somewhere in transitive dependencies.

Can you try to re-roll your lock file to get latest deps? If issue persists with v3, then there is some another dependency that may need a bump to use v1 of @embroider/* across the board

pjcarly commented 2 years ago

ember-auto-import v1 is resolved to v1.12.1. (coming from dependency ember-intl v6.0.0-beta.3)

After further investigation it seems that there are still @embroider/* dependencies under v1 (coming from ember-power-select and ember-basic-dropdown.

However the weird thing is, that everything is building just fine (with EPS and EBD), as long as ember-cli-mirage isn't added to the dependencies

¯\_(ツ)_/¯

kpfefferle commented 2 years ago

We're running into this same issue with our applications when trying out the v3-alpha release. I've tried forcing an @embroider/macros resolution in both directions (down to 0.50.x and up to 1.x) and neither is compatible.

If ember-cli-mirage releases a breaking v3 that requires @embroider/macros@1.x, then it seems that it's going to be a huge blocker to adoption. Maybe consider keeping @embroider/macros on an older version until there's a clear upgrade path that doesn't break everyone's builds?

cah-brian-gantzler commented 2 years ago

This is an issue everyone is having. It shows different error messages but it is the same error. Please see https://github.com/FortAwesome/ember-fontawesome/issues/194 and https://github.com/embroider-build/embroider/issues/1077

The second link thinks that the error actually happened with .41 of macros (and not sure why it didnt start being an issue til .50)

This issue states the problem happened in this repo on version 2.3 Looking at the yarn lock of 2.3 we included .41 of macros. https://github.com/miragejs/ember-cli-mirage/blob/v2.3.0/yarn.lock#L1821 It was updated as a result of updating the package.json. If this was the catalyst, its still pre .50. Would have to test a failing build with a fork of 2.3 and revert macros back to .40

I personally am using ember-cli-mirage 2.4 and not having the issue, but thats not the case for the person who opened the issue. Im currently fighting this exact issue, but with the update to ember-fontawesome.

Much like the ember-cli-babel 7.26 issue, It would seem the way forward is to get every addon to update to macros 1.0 as soon as possible.

Im really looking to embroider/macros to tell what what the way forward is.

kpfefferle commented 2 years ago

@cah-briangantzler My point is that ember-cli-mirage opted into forcing this issue by upgrading the @embroider/macros dependency when it worked fine before as it was. If that upgrade is rolled back, it could open the door to many more people opting into the 3.0-alpha series and being able to provide feedback on the changes this addon is making itself.

I agree that this is on @embroider/macros to provide a path forward, but ember-cli-mirage isn't required to force the pain point in the meantime by upgrading that dependency.

SergeAstapov commented 2 years ago

@kpfefferle sorry but I don't agree with you. By opting into using @embroider/macros this addon becomes Embroider compatible.

We can rollback the change adding @embroider/macros in v2 but I'm solid we need to use @embroider/macros in v3 to move forward and allow people to safely use ember-cli-mirage with Embroider.

There is nothing wrong with @embroider/macros specifically, it's just ecosystem needs to catch up and IMO majority of addons have already been updated to use @embroider/macros@1.x and if there is somewhere miss I'd recommend to help with those specific addons updates.

cah-brian-gantzler commented 2 years ago

Edit: LOL. See @SergeAstapov replied at the same time. Glad to see we are both on the same page

That is exactly why we opted into in V3, its a breaking change. If we dont for V3 when we do it would have to be V4. Currently there is nothing new in V3 just removal of deprecations following suit with the way ember does things.

Given that, it seems this is the perfect time to opt into it. Other addons have already updated, the one link if you read has Ed saying spread the word. The only way we are going to get past this hurdle is for addons to update. We have updated on a version V3 that is feature equivalent to V2. We are not forcing a pain point, we are giving you the oppertunity to stay on 2.4 til a way forward is given. If all the other addon you use also update to macros 1, then you can update to the V3 of this addon. By not providing this I would think we are causing a pain point for those that are trying to get over that hurdle.

As I stated earlier Im still running 2.4 for the same reason, my own apps are also blocked due to this issue.

Lets hope a way forward is given. As of now its seems to be an update all your addons at the same time.

SergeAstapov commented 2 years ago

@kpfefferle @cah-briangantzler more thinking of this, more I convinced we should rollback (=remove) @embroider/macros in v2 to not cause churn for people with < 1.0.0 version we use in ember-cli-mirage@2.

I'll open PR once I have a chance and release patch release v2.4.1 without @embroider/macros.

ember-cli-mirage@3 will stay on @embroider/macros@1. Note that we can't use @embroider/macros@1 in ember-cli-mirage@2 due to Node.js version requirements.

kpfefferle commented 2 years ago

Currently there is nothing new in V3 just removal of deprecations following suit with the way ember does things.

Sure seems like this @embroider/macros dependency bump is something new 🤔

I'd love to be able to opt into the removal of the deprecated features without breaking my build 🤷‍♂️

By opting into using @embroider/macros this addon becomes Embroider compatible.

Ok, then 3.0 is not just a deprecation removal release.

kpfefferle commented 2 years ago

For anyone else struggling with this, I was able to get things working by forcing @embroider/macros to resolve to a single version in my package.json:

// package.json
{
  // ...
  "resolutions": {
    "@embroider/macros": "1.2.0"
  },
  // ...
}

This seems to work with a number of different possible @embroider/macros versions as long as I rm -rf node_modules before yarn install to ensure conflicting versions are not hanging around and causing issues.

cah-brian-gantzler commented 2 years ago

@SergeAstapov if you are rolling back macros in V2 (and I think you should), I would stick with .40 as that was in 2.2 and people are reporting when we went to .41 in 2.3 is started causing issues

@kpfefferle I had tried this for my upgrading to ember-fontawesome and it did not work for me. However macros was at 1.0.1 at the time. Perhaps 1.2 has fixed the issue, will try it again.

cah-brian-gantzler commented 2 years ago

@kpfefferle I tried the resolutions again and this time it worked. I think I neglected to do the rm -rf node_modules last time. Thanks for the tip.

pjcarly commented 2 years ago

Honestly. I don't understand anything about embroider/macros and how it is causing the issues.

I have many years of Ember experience, but this is a complete mystery to me. Is there any documentation I can read up on to get a better understanding?

hoIIer commented 2 years ago

I've just run into this both when installing ember-cli-mirage in a fresh 4.2.0 project, and when installing the latest ember-fontawesome.

Is there any path forward without requiring npm-force-resolutions?

ef4 commented 2 years ago

I have many years of Ember experience, but this is a complete mystery to me. Is there any documentation I can read up on to get a better understanding?

The high level documentation for "what is @embroider/macros?" is in RFC 507

The particular reason there is breakage here is that the macros need to share one global configuration, and there was a bug in old versions that can make them choke on certain kinds of configuration, and some newer addons are using @embroider/macros to set that kind of configuration.

It is safe (in fact, much safer, hence this bug) to use yarn resolutions or NPM overrides to force all copies of @embroider/macros up to ^1.0.0. That gets you off the broken pre-1.0 versions.

The path of least resistance to fixing this particular issue cleanly is for ember-cli-mirage to do a major release dropping Node 10 support, without other breaking changes, so that it's easy for users to upgrade. Node 10 is Very Dead. Even Node 12 has only a month of security patches remaining.

rgaiacs commented 10 months ago

For anyone visiting this issue, I was having the reported issue with a fresh new project using Node 20.5.1, Ember 5.3.0, and ember-cli-mirage 2.4.0:

$ ember new mwe
$ cd mwe
$ ember install ember-cli-mirage
$ ember serve

Upgrade ember-cli-mirage to 3.0.0-alpha resolved the issue.

olenderhub commented 9 months ago

Upgraded from 2.1.0 (I had this problem in 2.2.0) to 3.0.0 (https://github.com/miragejs/ember-cli-mirage/releases/tag/v3.0.0) and it works to me (https://www.ember-cli-mirage.com/docs/getting-started/upgrade-guide#3-0-upgrade-guide)

SergeAstapov commented 9 months ago

Path forward for anyone having such issue - upgrade to v3.