Does this work with Embroider?

[tansongyang]

We're testing out Embroider in our application. With latest ember-modal-dialog@3.0.1, we were able to build, but there's an error at runtime:

Error: Assertion Failed: Could not find component named "ember-modal-dialog/-basic-dialog" (no component or template with that name was found)
    at assert (index.js:172)
    at RuntimeResolver.lookupComponent (index.js:8469)
    at DefaultRuntimeResolver.lookupComponent (runtime.js:1871)
    at resolveComponent (runtime.js:2358)
    at Object.evaluate (runtime.js:3177)
    at AppendOpcodes.evaluate (runtime.js:2008)
    at LowLevelVM.evaluateSyscall (runtime.js:4927)
    at LowLevelVM.evaluateInner (runtime.js:4883)
    at LowLevelVM.evaluateOuter (runtime.js:4875)
    at JitVM.next (runtime.js:5824)

[ChristopherConnock]

when staticComponents is set to true, I get a more specific error:

Build Error (PackagerRunner) in node_modules\ember-modal-dialog\templates\components\modal-dialog.hbs

Module Error (from C:/Users/chris/Documents/GitHub/roast-ember/node_modules/@embroider/webpack/src/webpack-hbs-loader.js):
Unsafe dynamic component modalDialogComponentName in $TMPDIR\embroider\792ec2\node_modules\ember-modal-dialog\templates\components\modal-dialog.hbs

[zion03]

With staticHelpers: true also

Module Error (from /app/node_modules/@embroider/hbs-loader/src/index.js):
Missing component or helper: ember-tether in node_modules/ember-modal-dialog/templates/components/tether-dialog.hbs

[gilest]

Yes with staticHelpers: true it seems you must install the ember-tether package even if you don't use tethering.

[RobbieTheWagner]

I'm seeing Unsafe dynamic component: this.modalDialogComponentName in $TMPDIR/embroider/4a2375/node_modules/ember-modal-dialog/templates/components/modal-dialog.hbs/modal-dialog.hbs

[RobbieTheWagner]

@lukemelia are there any plans to make this addon embroider and/or Ember 4 compatible?

[lukemelia]

@rwwagner90 I definitely intend for it to be compatible with Embroider and Ember 4. I haven't gotten there on my apps yet, so haven't been able to prioritize that work, but I am happy to support anyone who wants to take it on.

[lukemelia]

This should be fixed in 4.0.0-beta.0, thanks to @rwwagner90

[RobbieTheWagner]

The issue with ember-tether may persist. I think we'll have to add it as a peer dep. Will open a PR for that now.

[RobbieTheWagner]

#371

[lukemelia]

As of 4.0.0, it should be compatible with Ember 4 + Embroider

[RobbieTheWagner]

@lukemelia I think we still need #371 or people will all have to install ember-tether themselves

[lukemelia]

@rwwagner90 I started looking at #371, and in order to have test coverage working well with optional peer dependencies, I think we'll need to move to the v2 addon monorepo pattern, with a test app that has optional deps installed and one that doesn't. I created #373 to track.

[RobbieTheWagner]

@lukemelia perhaps I am misunderstanding how it works, but can't we just ship it as a normal peerDependency? The users will get a warning that they need to install it, but they shouldn't actually have to, I think.

[gilest]

test app that has optional deps installed and one that doesn't

@lukemelia can you achieve this with ember-try and @embroider/macros? Curious because I'm thinking of doing something similar

[lukemelia]

@lukemelia can you achieve this with ember-try and @embroider/macros? Curious because I'm thinking of doing something similar

I don't see any reason that wouldn't work.