adopted-ember-addons / ember-stripe-elements

A simple Ember wrapper for Stripe Elements
https://ember-stripe-elements.netlify.com
MIT License
19 stars 23 forks source link

Dynamic import from app initializer #40

Closed jaswilli closed 2 years ago

jaswilli commented 2 years ago

Hi,

Do we know for certain that the dynamic import from the app initializer is supported by ember auto import and that it's working as expected? I'm trying to move an app over from the old version of this addon to the one here (in the adopted-ember-addons namespace) and so far I've been unable to get tests working because when the app boots up it blows up on the dynamic import in the app initializer (https://github.com/adopted-ember-addons/ember-stripe-elements/blob/master/app/initializers/ember-stripe-elements.js) like so:

not ok 1 Chrome 98.0 - [44 ms] - Integration | Component | stripe-wrapper: it renders
    ---
        actual: >
            null
        stack: >
            Error: Could not find module `_eai_dyn_@adopted-ember-addons/ember-stripe-elements/test-support` imported from `(require)`
                at missingModule (http://localhost:7357/assets/vendor.js:259:11)
                at findModule (http://localhost:7357/assets/vendor.js:270:7)
                at requireModule (http://localhost:7357/assets/vendor.js:36:15)
                at module.exports.window.emberAutoImportDynamic (webpack://__ember_auto_import__/../../../../private/var/folders/0t/y4v08nbs6kd59z_8n4vb0j300000gn/T/broccoli-68533uKt5Eb1uguRF/cache-297-webpack_bundler_ember_auto_import_webpack/tests.js?:7:14)
                at Object.initialize (http://localhost:7357/assets/stripetest.js:361:7)
                at http://localhost:7357/assets/vendor.js:31475:21
                at Vertices.each (http://localhost:7357/assets/vendor.js:53500:9)
                at Vertices.walk (http://localhost:7357/assets/vendor.js:53414:12)
                at DAG.each (http://localhost:7357/assets/vendor.js:53344:22)
                at DAG.topsort (http://localhost:7357/assets/vendor.js:53352:12)
        message: >
            Promise rejected before "it renders": Could not find module `_eai_dyn_@adopted-ember-addons/ember-stripe-elements/test-support` imported from `(require)`

I'm using dynamic import elsewhere in the app in question so I'm pretty sure I have things configured correctly. Also, here's a minimal reproduction of what I'm seeing: https://github.com/jaswilli/stripetest.

Of note is that if I remove the initializer and just import StripeMock from '@adopted-ember-addons/ember-stripe-elements/test-support'; from within a test and assign StripeMock to window from within a beforeEach hook things work as expected, so I think the test helpers themselves are fine.

According to this issue https://github.com/ef4/ember-auto-import/issues/488 ember auto import is unable to dynamically import libraries through a v1 addon, which as far as I know is what we're seeing here. It's completely possible I'm misunderstanding some nuance though.

Assuming I'm not missing something and this is actually broken, should we remove the initializer and instead document how to wire up the StripeMock inside a test hook?

st-h commented 2 years ago

Thanks for the report and the details. Personally, I almost had removed the initializer, however I didn't because I wasn't sure if this would make upgrading unnecessarily difficult. Yet, I didn't expect it to break in such a way. Could you please submit a PR including instructions how to set up tests without the initializer?

esbanarango commented 2 years ago

I'm having the same issue here:

node 14.19

"ember-source": "3.28.1"
"@adopted-ember-addons/ember-stripe-elements": "2.0.2"
"ember-auto-import": "2.4.0",
st-h commented 2 years ago

@esbanarango thanks for the report. All I can offer at the moment is to release a version without the initialiser. However, that would put the burden of how to figure out test support on everyone who needs test support. Therefore I would prefer to include an upgrade path (at least in the form of a section in the README). However, at the moment I have very limited time to figure that out and write that down. So, if someone can submit a PR, that would help speed things up tremendously.

esbanarango commented 2 years ago

@st-h here it is: https://github.com/adopted-ember-addons/ember-stripe-elements/pull/41

jaswilli commented 2 years ago

@esbanarango Thanks for picking this up, I hadn't had a chance to get back to it yet.

st-h commented 2 years ago

Thanks a lot. Released as 2.0.3