Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use native-dom-helpers internally #311

Closed
knownasilya opened this issue May 19, 2017 · 9 comments
Closed

Use native-dom-helpers internally #311

knownasilya opened this issue May 19, 2017 · 9 comments
Labels

Comments

@knownasilya
Copy link

knownasilya commented May 19, 2017

https://github.com/cibernox/ember-native-dom-helpers

Relates to #256 and #289

Leaving this as a placeholder.
cc @san650 @cibernox

@san650 san650 added the feature label May 23, 2017
@san650
Copy link
Owner

san650 commented May 23, 2017

Thanks @knownasilya for opening this issue.

@ming-codes
Copy link

Well, just say the words on how you like to proceed. I'm sure there are people that like to contribute.

@simonihmig
Copy link
Contributor

Would also love to see this. Currently using e-c-p-o is the only reason we cannot drop jQuery from a new app we started with... 😥

One thing to keep in mind: all e-n-d-h helpers that trigger an event will return a promise (basically returning wait()), now usually used with await. This allows to get rid of some wait() / andThen() calls in the test itself, which is nice.

But for this to work with e-c-p-o, all "actions" like page.foo.click() would also have to return this promise then, and not like now this for the fluent interface. So this would need to be a breaking change!

@knownasilya
Copy link
Author

knownasilya commented Aug 11, 2017

You could still keep chainability, you'd just need to change the API to have an explicit method on the end to return the promise. But yes, it would be a breaking change.

@san650
Copy link
Owner

san650 commented Aug 12, 2017

@simonihmig

But for this to work with e-c-p-o, all "actions" like page.foo.click() would also have to return this promise then, and not like now this for the fluent interface.

Since the beginning we made all e-c-p-o nodes to behave like a promise so you can use await (or .then) with actions.

All the following are supported right now

const page = create({
  visit: visitable('...'),
  foo: clickable('...'),
  bar: clickable('...'),
});

await page.visit();
await page.visit().foo();
await page.visit().foo().bar();
await page.visit().foo().then(() => ...);
...

This is because of this

const thenDescriptor = {
isDescriptor: true,
value() {
return (window.wait || wait)().then(...arguments);
}
};

and this

then: thenDescriptor,

which is injected in all page objects nodes.

@san650
Copy link
Owner

san650 commented Aug 12, 2017

Would also love to see this. Currently using e-c-p-o is the only reason we cannot drop jQuery from a new app we started with... 😥

One approach we want to take is to make jQuery a direct dependency of this addon and only include it in tests, allowing users not to include jQuery on their app bundle. Removing jQuery entirely from the addon will require a lot of effort and break existing features... but we'll do it eventually.

We want to start this migration by replacing the events first by using native-dom-helpers addon.

@simonihmig
Copy link
Contributor

Since the beginning we made all e-c-p-o nodes to behave like a promise so you can use await (or .then) with actions.

Oh, didn't know, that's brilliant!

One approach we want to take is to make jQuery a direct dependency of this addon and only include it in tests, allowing users not to include jQuery on their app bundle.

Not so sure about this one. Initially also thought about this approach when getting rid of jQuery from ember-bootstrap, because refactoring all the tests seemed to be most of the work. But you end up testing your code in a environment that is substantially different than production, so if you still have some jQuery usage left over, this will still work in tests, but break in the real world. (So I ended up making https://github.com/simonihmig/ember-native-dom-helpers-codemod)

This is true even more so for apps, where it is easy to get some (even nested) addon require jQuery without you necessarily knowing about it. So tests pass, but app is broken (without jQuery).

Just wondering, but maybe there is a way to use it just internally and hide it from app code, so Ember.$ and this.$() are undefined?

@ro0gr
Copy link
Collaborator

ro0gr commented Aug 20, 2017

Currently using e-c-p-o is the only reason we cannot drop jQuery from a new app we started with... 😥

on the same boat

Just wondering, but maybe there is a way to use it just internally and hide it from app code, so Ember.$ and this.$() are undefined?

I've tried:

$ npm i --save jquery
// index.js
module.exports = {
  name: 'ember-cli-page-object',

  options: {
    nodeAssets: {
      // ...
      jquery: {
        import: ['dist/jquery.js'],
      }
    }
  },

It makes Ember.$ an undefined. Also import $ from 'jquery' works. Working on a pr currently

@ro0gr
Copy link
Collaborator

ro0gr commented Nov 15, 2017

@san650 just published v1.13.0-alpha.1 which includes native-dom-helpers support. Feel free to test and report.

Docs in progress #334

@san650 san650 closed this as completed Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants