machty / ember-concurrency

ember-concurrency is an Ember Addon that enables you to write concise, worry-free, cancelable, restartable, asynchronous tasks.
http://ember-concurrency.com
MIT License
689 stars 157 forks source link

Add `ember-glimmer` to locations where `INVOKE` symbol can be find. #312

Closed thiagofelix closed 4 years ago

thiagofelix commented 5 years ago

The PR #67 fixed the INVOKE symbol for glimmer2 by adding 'ember-glimmer/helpers/action' as one possible location where such constant can be find, starting on ember@3.1.0 this location is no longer valid.

I was able to trace the ember PR #16291 that stoped exporting the INVOKE symbol from the file above.

The correct place to find the INVOKE symbol now should be from the ember-glimmer package (root package, not the sub module).

The problem I am having is trying to use ember-concurrency with ember@3.1.x version, the INVOKE symbol is not being found.

Ember Twiddle displaying the problem:

Issue with ember@3.1.0: https://ember-twiddle.com/9fcee21b1dc407e4d61139bd7b2858b5

Working well with ember@3.0.0: https://ember-twiddle.com/3e8869d9ca19ee96305a152d2f07d596

thiagofelix commented 5 years ago

Another check that led me to create this PR was comparing the ember@3.0 and ember@3.1 final bundles from npm: https://unpkg.com/ember-source@3.1.0/dist/ember.debug.js https://unpkg.com/ember-source@3.0.0/dist/ember.debug.js

On the 3.1 you can't find the package ember-glimmer/helpers/action so I assumed the ember-concurrency would fail bc of that.

thiagofelix commented 5 years ago

@maxfierke I am on it =)

maxfierke commented 5 years ago

@thiagofelix hey, just wanted to see if you might have a chance to take a look at adding those tests?

thiagofelix commented 5 years ago

Hey @maxfierke I am not really sure how/what we want to test here, can you advise pls? Basically the cause of the problem is related to which ember-glimmer version the user is using. In my mind we would need to run the test against some different ember-glimmer version and break if the INVOKE symbol is no longer found in one of them? Not entirely sure what exactly we want to assert given our test environment dependencies are stable/static 🤷‍♂

maxfierke commented 5 years ago

@thiagofelix I think we can get by with just a test that ensures you can pass a task to the action helper and have it triggered correctly. We don't need to get granular about testing for the exact import path. Probably a single integration test would do.