embroider-build / ember-auto-import

Zero config import from npm packages
Other
360 stars 109 forks source link

Can you make acceptance tests wait for dynamic import #264

Open mansona opened 4 years ago

mansona commented 4 years ago

We're using ember-auto-import and the dynamic import feature to reduce the bundle size of the new Ember website, but I've noticed something in our acceptance tests 🤔 It looks like tests aren't waiting for the dynamic import to complete before continuing i.e. await settled() doesn't seem to wait for the module to download

You can see the diff of this Percy doesn't have any charts because we're loading them dynamically now https://percy.io/Ember/Ember-Website/builds/3826777

Do you have any tips or tricks to make it wait for ember-auto-import?

ef4 commented 4 years ago

You're right, this is not handled automatically. The workaround is to treat your asynchrony as a black box and use waitFor or waitUntil, so that no matter what the component is waiting for the test will respect it.

But it would also be good to improve ember-auto-import so it registers a waiter so that settled() includes having no outstanding dynamic import requests. If you want to try it, it would be a relatively small change.

This is where you'd add code to count the number of outstanding requests. It would change to something like:

var r = _eai_r;
var outstandingRequests = 0;
window.emberAutoImportDynamic = function(specifier) {
  outstandingRequests++;
  return r('_eai_dyn_' + specifier).then(function(mod){ 
    outstandingRequests--;
    return mod;
  }, function(err) {
    outstandingRequests--;
    throw err;
  });
};

And you would use registerWaiter to wait until outstandingRequests is zero.

Probably all of this should only happen when not in a production build.

simonihmig commented 4 years ago

Seems like a duplicate of #217 :)

simonihmig commented 4 years ago

In case this is helpful for others: we often have a custom waitFor decorator in our code bases that wraps the test waiter stuff around any async method, so you can do something like this:

@waitFor
async loadFoo() {
  return import('foo'); // or any other async stuff like fetch
}

ember-test-waiters has some more reusable abstraction than our ad-hoc implementation, and decorator support seems also in the works: https://github.com/emberjs/ember-test-waiters/pull/131

ef4 commented 4 years ago

If somebody wants to try to fix this, it requires using ember’s registerWaiter and adding code to the emberAutoImportDynamic function.

On Fri, May 8, 2020 at 4:33 AM Simon Ihmig notifications@github.com wrote:

In case this is helpful for others: we often have a custom waitFor decorator in our code bases that wraps the test waiter stuff around any async method, so you can do something like this:

@waitFor async loadFoo() { return import('foo'); // or any other async stuff like fetch }

ember-test-waiters https://github.com/emberjs/ember-test-waiters has some more reusable abstraction than our ad-hoc implementation, and decorator support seems also in the works: emberjs/ember-test-waiters#131 https://github.com/emberjs/ember-test-waiters/pull/131

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ef4/ember-auto-import/issues/264#issuecomment-625706378, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACN6MXO4HH573WWLWYDOITRQO7W5ANCNFSM4KIQLGOQ .

rwjblue commented 3 years ago

Just to mention RE: @simonihmig's example, @waitFor is now provided directly by @ember/test-waiters (and works for both generator functions and async functions).

Mikek2252 commented 3 years ago

Hey i'm looking in to implementing this, but i'm not too sure how i get registerWaiter within this section https://github.com/ef4/ember-auto-import/blob/e854e2e5f90333fd5816ae8f39a64162912526e3/packages/ember-auto-import/ts/webpack.ts#L39 I tired registering it with registerHelper and importing it with

import { registerWaiter } from '@ember/test'; but it then throws this error: Cannot find module '@ember/test' or its corresponding type declarations. Can anyone help push me in the right direction?

ef4 commented 3 years ago

Are you putting the import inside the entryTemplate string, or outside the string? I'd expect that error if outside.

Mikek2252 commented 3 years ago

At first i tried putting it inside the string and got this

ERROR in ../../../../../private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-77889vZb3XfrUhmgD/cache-146-bundler/staging/app.js 2:0-45
Module not found: Error: Can't resolve '@ember/test' in '/private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-77889vZb3XfrUhmgD/cache-146-bundler/staging'

so i tried passing it in from the ts doc.

Mikek2252 commented 3 years ago

my diff for reference https://github.com/ef4/ember-auto-import/compare/master...Mikek2252:master

Mikek2252 commented 3 years ago

@ef4 Hey i'm having another look at this and having trouble getting import { registerWaiter } from '@ember/test'; to resolve correctly. I though i might need to create some form of alias for @ember/test to point it to ember-source but not having to much luck. Have you got any ideas?

ERROR in ../../../../../private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-18001ehKakLUicQwS/cache-146-webpack_bundler_ember_auto_import_webpack/tests.js 2:0-65
Module not found: Error: Can't resolve '@ember/test' in '/private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-18001ehKakLUicQwS/cache-146-webpack_bundler_ember_auto_import_webpack'
resolve 'ember-source/dist/ember-testing' in '/private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-18001ehKakLUicQwS/cache-146-webpack_bundler_ember_auto_import_webpack'
  Parsed request is a module
  No description file found in /private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-18001ehKakLUicQwS/cache-146-webpack_bundler_ember_auto_import_webpack or above
  resolve as module
    /private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-18001ehKakLUicQwS/cache-146-webpack_bundler_ember_auto_import_webpack/node_modules doesn't exist or is not a directory
    /private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/broccoli-18001ehKakLUicQwS/node_modules doesn't exist or is not a directory
    /private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/T/node_modules doesn't exist or is not a directory
    /private/var/folders/0p/qgbj4c3d02s8xrmj556jbx780000gn/node_modules doesn't exist or is not a directory
    /private/var/folders/0p/node_modules doesn't exist or is not a directory
    /private/var/folders/node_modules doesn't exist or is not a directory
    /private/var/node_modules doesn't exist or is not a directory
    /private/node_modules doesn't exist or is not a directory
    /node_modules doesn't exist or is not a directory