[WIP] Chore/update ember-cli to 3.0.0

[jacobq]

No description provided.

[jgwhite]

Thanks for taking on this task. Let us know when you’d like us to have a look 👍

[jacobq]

@jgwhite Unfortunately I ran out of time to finish this (underestimated my lack of experience with ember testing, especially the new API). Could you help me figure out how to import / reference the custom test helpers this addon defines (drag and reorder)? See FIXME comment in https://github.com/jgwhite/ember-sortable/pull/177/files#diff-c1ef8f3d7d1851c001639465c61178b6

There were also some things that looked just plain wrong in the tests. For example, the test to check that 'isDragging' / 'isDropping' get set to false (during willDestroyElement hook) actually calls destroy() which prevents willDestroyElement from getting a chance to change them (since the object is already destroyed rather than just scheduled for destruction the calls .set(...) result in an error). I changed it to use clearRender() instead of destroy(), so that one should be good, but it makes me wonder if maybe the test suite is just a bit out of date and could use some ❤️ regardless of my original goals of this PR (i.e. to update ember-cli and eliminate a deprecation warning).

[jgwhite]

Could you help me figure out how to import / reference the custom test helpers this addon defines (drag and reorder)?

I think, due to the way the test-support tree is merged into the build, it’ll need to be:

import { drag, reorder } from 'dummy/tests/helpers/ember-sortable';

There were also some things that looked just plain wrong in the tests. For example, the test to check that 'isDragging' / 'isDropping' get set to false (during willDestroyElement hook) actually calls destroy() which prevents willDestroyElement from getting a chance to change them (since the object is already destroyed rather than just scheduled for destruction the calls .set(...) result in an error). I changed it to use clearRender() instead of destroy(), so that one should be good, but it makes me wonder if maybe the test suite is just a bit out of date and could use some ❤️ regardless of my original goals of this PR (i.e. to update ember-cli and eliminate a deprecation warning).

Wow, yeah. That test is super, super old and I’m not surprised it broke. Your solution sounds good to me. You’re right the tests could do with some spring cleaning.

Are you attending EmberConf?

[jgwhite]

Sorry, that should have been:

import { drag, reorder } from 'dummy/tests/helpers/ember-sortable/test-helpers';

Which, now that I look at it, is pretty verbose. Wonder if we can make the path shorter?

[jacobq]

Unfortunately, I won't be attending EmberConf (in person) this year, though I do hope to go next year. This is a busy month for me, though I do intend to spend a little time working on this again next week.

[validkeys]

@jacobq I was able to resolve the unit tests w/ the new testing system like so: https://github.com/jgwhite/ember-sortable/blob/8d47a522aeed0bb37622ea9125c47dc3f0bfe2c2/tests/unit/components/sortable-item-mixin-test.js

Essentially, since the unit test requires a DOM element:

  hooks.beforeEach(async function() {
    const mockComponent = Component.extend(SortableItemMixin, {
      classNames: ['test-mock-cmpt'],
      didInsertElement() {
        this._super()
        subject = this
      }
    })
    await run(async () => {
      this.owner.register('component:test-mock', mockComponent)
      group = MockGroup.create();
      this.set('group', group)
      this.set('rendered', true)
      await render(hbs`{{#if rendered}}{{test-mock group=group}}{{/if}}`)
      // run(() => subject.set('group', group))
    });
  });

Render the mixin in an empty component. In the didInsertElement method on the component def, simply set a local variable subject to the component's context. That way you get the component lifecycle hooks with access to the inner state of the component

I opened a separate PR for upgrading to 3.4.2. Let me know if you have any thoughts.

[jacobq]

@validkeys Hey, thanks for working on this! I've got another ember-cli/deps update chore PR to do later this month so will plan to look at this again too. I suspect that another LTS release of Ember will be coming soon too, so so maybe we'll get two birds with one stone.

[jgwhite]

This project is now being maintained by the team at @heroku.

To get the project healthy and maintainable again, we’re doing a bit of spring cleaning of older issues and pull requests. This pull request seems like it will be superseded by subsequent PRs so we’re closing it. Thank you for all the hard work you put in, the ideas and solutions will live on in other PRs.

Please feel free to re-open if you’d like to resume work on it.

[jacobq]

@jgwhite Oops, I forgot all about this! No problem. I'm glad someone is tidying up, and the updates will probably make this irrelevant / unnecessary anyway.