Backwards compatibility break v0.9.8 -> master: selectors passed into custom commands

[senocular]

When a custom command (non-page command) is used from a page object using a @-named element reference, the v0.9.8 behavior will pass in the selector string when the new (current master) behavior passes in the element object.

// custom-commands/customCommand.js
exports.command = function (selector) {
  this.perform(function () {
    console.log('Selector:', selector); // see outputs below...
  });
};

// page-objects/myPageObject.js
module.exports = {
    elements: {
        simple: '.simple'
    },
    commands: []
};

// tests/test.js
module.exports = {
  test: function (browser) {
    browser.page.myPageObject().customCommand('@simple');
  },

  'after': function (browser) {
    browser.end();
  }
}

Current release (v0.9.8) output:

Selector: '.simple'

Current master output:

Selector: { name: 'simple',
  selector: '.simple',
  locateStrategy: 'css selector',
  parent:   { ...
}

If you're just passing this value along to native commands, its fine. But if you're doing more, for example expecting this to be a string for some other usage, then you're going to have problems.

We had a custom command which passed this value along to an execute() which produced an Error while running execute command: Converting circular structure to JSON error because it was no longer passing a simple string, instead passing the entire element object which was causing some havoc.

While a compatibility break, this behavior is a necessary evil to allow the element's own locateStrategy to be passed along with the selector without affecting the global locateStrategy which is part of the encapsulation capabilities of the feature.

Workaround:

// custom-commands/customCommand.js
exports.command = function (selector) {
  // for old behavior:
  selector = typeof selector === 'object' ? selector.selector : selector;
  // ... and maybe call useCss() or useXpath() if going between those strategies based on selector
  // ...
};

[beatfactor]

I'm not sure if this is a very common use case. I imagine that non-page custom commands aren't commonly used with @-named elements, instead I expect that page object custom commands are used like that. Are only non-page custom commands affected? Still, we shouldn't introduce breaking changes, unless it's a major release (1.0 release). Clearly this cannot be a major release, so we need to patch it to be backwards compatible. Isn't it possible to resolve the @-named string into the selector object only if it's needed (using Element.requiresFiltering)?

EDIT: I can see this is not trivial, I'll look into it and see if it's worth the effort.

[senocular]

I don't think its possible feasible. If the selector is reduced to a string, its a potential break for the locate strategy [unless the old code path is restored for that use case]. This (these) changes will probably just have to be shelved until we're ready to allow some breaking changes.

[diachedelic]

Page objects break custom assertions for me (selector is an object not a string):

exports.assertion = function(selector, count) {
  this.message = 'Testing if element <' + selector + '> has count: ' + count
  this.expected = count
  this.pass = function(val) {
    return val === this.expected
  }
  this.value = function(res) {
    return res.value
  }
  this.command = function(cb) {
    var self = this
    return this.api.execute(function(selector) {
      return document.querySelectorAll(selector).length
    }, [selector], function(res) {
      cb.call(self, res)
    })
  }
}

Is this expected behaviour? How do I rewrite that custom assertion to support page objects?

[diachedelic]

I added this at the start of the method:

exports.assertion = function(selector, count) {
  if (Array.isArray(selector)) {
    // combine page object CSS selectors
    selector = selector.map(s => s.selector).join(' ')
  }

  ...
}

[Akaryatrh]

Hi all, seems this issue is back with v1.x.x

Still I don't get your fix @diachedelic? The type of selector is object, I can't see how It would enter your condition?

[diachedelic]

@Akaryatrh can't remember - perhaps i meant to say Array instead of object?

Edit: looks like I'm handling an array of objects

[Akaryatrh]

Maybe. Anyway, I'm moving away from v1.X.X, I encountered too much errors, I guess It's better to wait for a migration guide ^^

BTW, thx for your quick answer!

[aberonni]

With the latest v1.0.14 this original error is not encountered anymore:

We had a custom command which passed this value along to an execute() which produced an Error while running execute command: Converting circular structure to JSON error

Now the behaviour has been changed with this new release, and I have documented this breaking change here: https://github.com/nightwatchjs/nightwatch/wiki/Migrating-to-Nightwatch-1.0#custom-commands

I think we can now close this issue.

[qooban]

@aberonni
What about selectors and sections? I find it problematic in the latest (1.0.18) version.

I have such Page Object - page:

module.exports = {
    url: function () {
        return this.api.launch_url + '/page/0';
    },

    sections: {

        (...truncated...)

        userMenu: {
            selector : '.usersMenu',
            elements: {
                userName: 'span:first-child',
                userDropdownMenu : '.menu',
                editModeMenuItem : '#editModeMenuItem',
                resetMenuItem : '#resetMenuItem',
                logoutMenuItem: '#logoutMenuItem'
            },
            props: {
                editModeLabel: 'Edit Mode',
                exitModeLabel: 'Exit Edit Mode'
            }
        },

and using such custom command - clickElement:

exports.command = function(selector) {
    return this
        .waitForElementVisible(selector)
        .click(selector, (result) => this.log('Clicked element', result.status === 0 ? 'successfully.' : 'with no success. error - '+result.value.message));
};

from such test file:

var Config = require('../config');

module.exports = {
    'Admin user menu': function (client) {
        client.login()
            .page.page().section.userMenu
            .assert.containsText('@userName', Config.admin)
            .clickElement('@userName')
            .waitForElementVisible('@userDropdownMenu')
            .assert.containsText('#resetMenuItem span','Reset Templates')
            .assert.containsText('#editModeMenuItem span','Edit Mode')
            .assert.containsText('#logoutMenuItem span','Logout');

        client.end();
    },

and it looks like .clickElement('@userName') is called on span:first-child and not .usersMenu span:first-child, so it is not calculated.

It was working properly (selector was calculated according to section's selector) in v0.9.21.

[beatfactor]

@qooban could you open a separate issue please regarding this? Make sure to post the verbose log as well, thanks.