v3 features

[rosshinkley]

I'm not sure where to put this, and I'm open to this being moved to a Wiki if that makes more sense. In the interest of having all of the big (and possibly breaking) features/nice-to-haves for the next big release in one spot:

• Native iframe support and management - Interacting with elements inside an iframe #203, Enable support for IFrames, fixes #203 #496
• Multiple window management support (including popups) - Handling pop-up windows #352, Access the popup opened by the app #510
• IPC safety - Simultaneous calls to the same action can cause early returns/continues with the wrong results #493, Callback-based IPC #579
• Queue safety - Simultaneous calls to the same action can cause early returns/continues with the wrong results #493, Polymer v1.0 #239, Problems using nightmare in a loop #522 (and many, many more)
• Actions with selectors waiting on selector prior to executing action - wait and click #388
• .goto() optionally returning after DOMContentLoaded
• Reduce plugin function complexity

There may be a need for:

• An upgrade guide for users
• Splitting up the documentation (akin to how Electron has it)
• A semi-formal statement of ongoing support for 2.x for some period once 3.x is finished

Nice to-haves:

• Manipulating elements with custom predicates - Support custom element predicates #867

What have I missed in features, needs, or linked issues?

edit: added element

[Mr0grog]

#596 makes me wonder if changing viewport() to actually set the viewport (as opposed to window) size would be a good breaking change for v3.

[Mr0grog]

Maybe have goto return as soon as a page is loading so you can execute scripts ASAP during load?

Have actions with selectors wait for the selector before doing the action (#388). Capybara (Ruby), for example, does this. It could possibly be configurable (though I think you’d want opt-out rather than opt-in).

[rosshinkley]

IPC safety will likely make it into the next release of Nightmare.

[rosshinkley]

#596 makes me wonder if changing viewport() to actually set the viewport (as opposed to window) size would be a good breaking change for v3.

Hm, how are you planning on doing that? Using window.outerWidth and outerHeight and then adding the difference? (I don't remember being able to set the viewport directly, but I'm probably wrong.)

Maybe have goto return as soon as a page is loading so you can execute scripts ASAP during load?

To what end? Should preload be extended (or pluggable) instead?

Have actions with selectors wait for the selector before doing the action (#388). Capybara (Ruby), for example, does this. It could possibly be configurable (though I think you’d want opt-out rather than opt-in).

Fully on-board for an implementation of #388.

[Mr0grog]

how are you planning on [changing the viewport size instead of window size]?

I think just calling window.setContentSize() instead of window.setSize()

To what end? Should preload be extended (or pluggable) instead?

I think I was thinking so you could get in as early as DOMContentLoaded or maybe before other scripts, but maybe specifying preload stuff somehow would be better. Not an idea I’d thought through deeply.

[rosshinkley]

I think just calling window.setContentSize() instead of window.setSize()

Of course. Y'know, I've read the browser-window docs a zillion times and forgot about setContentSize. Maybe I need to read it a few more times.

I think I was thinking so you could get in as early as DOMContentLoaded or maybe before other scripts, but maybe specifying preload stuff somehow would be better.

I was pondering out loud, and now I'm wondering if plugging preload is a good idea. I think preload is executed prior to every page load, in so doing losing a bit of control - there are probably situations where you want to run a given script on a given page's load.

I doubt I'm thinking about it the way you are: what would you suggest as API changes? It seems like you're advocating having a navigation setup method and a navigation method - am I anywhere close to being on the same page?

[rosshinkley]

I've been tinkering with the idea of changing the Electron part of .action() to wrap the ambient require. The function signature is ugly: function(name, options, parent, win, renderer, done). Changing the .call in the action handler from:

.call({require: require, parent: parent})

...to something more like....

.call({
  require: (lib) => {
    if(lib==='nightmare-plugin') {
      return {
        name: name,
        options: options,
        parent: parent,
        win: win
      };
    }
    return require(lib);
  },
  parent: parent
})

... which would make the handler signature be function(done). Also, dropping the renderer as I think it could be required.

I realize wrapping require is also kind of ugly and probably unadvised, but I thought I'd float it out. Thoughts?

[Mr0grog]

I definitely agree that function signature is not only a little ugly, but very hard to get right. I have had to look it up in the README literally every single time I’ve needed to use it and I’ve gone so far as to write it like function (_, __, parent, ___, ____, done) in some examples because you really only need one or two of those most of the time.

But why not just make those variables ambiently available in the plugin’s closure instead of wrapping require? Or, barring that, have plugins take two args: an object with all the things (as you’d get from the require call in your example) and the done callback?

(Also agree that dropping renderer would be pretty reasonable because there are other ways to get it.)

[Mr0grog]

I was pondering out loud, and now I'm wondering if plugging preload is a good idea.

It’s probably not. I think my real desire, initially, was just to be able to run code before waiting for the whole page to fully finish loading all its resources (exactly how, in the client, you often want to run on DOMContentLoaded instead of on load). My use case is basically: “I want to click this button and I don’t care whether images have loaded yet.”

BUT the more I think about it, that makes a lot of things more complicated: have all the page’s scripts (if some where deferred) loaded yet? What if you just want to load a page and screenshot it? Now you have to load, wait, then screenshot. Etc. A possible alternative might just be to add an option to goto asking it to return as soon as the HTML is loaded, but keep the default behavior the same. (e.g. goto('url', {waitForResources: false}))

[rosshinkley]

I have had to look it up in the README literally every single time I’ve needed to use it

Me too. And I wrote the [expletive redacted] thing. I'm going to go on a limb and say it's the most unwieldy part of Nightmare, and humbly apologize.

But why not just make those variables ambiently available in the plugin’s closure instead of wrapping require?

It's certainly easy to do, but makes where those variables came from kind of opaque. The less it seems like magic, the better. (Using require only makes this marginally better, I realize.)

Or, barring that, have plugins take two args: an object with all the things ...

The options hash was frowned upon in the orginal incarnation. I wouldn't be totally against it (provided it has a descriptive name), but require seemed to satisfy both the "no options hash" and the "this function signature is impossible to remember" demi-requirements.

[rosshinkley]

My use case is basically: “I want to click this button and I don’t care whether images have loaded yet.”

That's what I was guessing, and that would be awkward to cram into preload. That's why I walked it back. I'm not sold that plugging preload is a bad thing necessarily (having to redefine all of the machinery for console et al is not a great solution, either), but not going to solve what you had in mind.

BUT the more I think about it, that makes a lot of things more complicated...

This is what I was thinking, but I thought maybe there was an obvious part that I had missed. (I have a penchant for overcomplication.)

A possible alternative might just be to add an option to goto asking it to return as soon as the HTML is loaded, but keep the default behavior the same. (e.g. goto('url', {waitForResources: false}))

I'm trying to think of countercases, and while I can think of instances where not waiting is silly, they are still valid use cases and I think would behave properly. (Eg, .goto(myUrl, {waitForResources: false}).wait('.mySelector') where .mySelector is added by the client.) At first blush, this is a reasonable approach.

[Mr0grog]

Generally: have every action return something? In a similar vein, be better about errors. Also: make all errors instances of Error (maybe a subtype, like NightmareError or something). This covers a lot of things, so probably requires an audit of all existing actions first (if it seems like a good idea).

[rosshinkley]

Generally: have every action return something? In a similar vein, be better about errors.

Can't +1 this enough.

maybe a subtype, like NightmareError or something

That's something I had not considered, but the idea is certainly appealing.

so probably requires an audit of all existing actions first (if it seems like a good idea)

This is probably worth doing. Added #646.

[overflowz]

Seriously, when this release will be available? Popups / New windows are giving me serious nightmares and struggling a lot 😭 I'm still using nightmare-window-manager, but sometimes it does not help (popup logins for example, triggered by javascript with anti csrf token + link of something, etc and all of these are obfuscated or really painful to dig in).

[rosshinkley]

@overflowz The main holdup right now is that preload is not honored by windows not created by instantiating a new BrowserWindow internal to Electron, specifically with window.open(). See electron/electron#2605.

I haven't dug into it, but it might be skirted with some preload magic on the main Electron window. At the time, it felt like an awful lot of work for something that might end up getting fixed.

[TimNZ]

Does handling webContents 'new-window' event solve the preload issue?

https://electron.atom.io/docs/api/web-contents/#event-new-window