-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 usage #661
Comments
I am guessing you're referring to What are you trying to do? |
I'm trying to build a module for dealing with
I don't like the idea of encouraging my users to patch the global |
In the development of I think Thinking out loud: would |
For what it's worth: I've thrown together a |
Nice! That helped me understand what I don't know if this should be encouraged at all but I've been extending the prototype manually so that I can get support for plugins in a nice way, not sure if I want to release anything using this though, but this is what it looks like:
function selectHasOption (selector, value) {
return this.evaluate((selector, value) => {
// ...
}, selector, value)
}
function selectHasOptionWithText (selector, text) {
return this.evaluate((selector, text) => {
// ...
}, selector, text)
}
// ...
module.exports = function (nightmare) {
nightmare.prototype.selectHasOption = selectHasOption
nightmare.prototype.selectHasOptionWithText = selectHasOptionWithText
// ...
}
const Nightmare = require('nightmare')
const NightmareSelect = require('nightmare-select')
const nightmare = new Nightmare()
nightmare.use(NightmareSelect)
await nightmare.goto('http://...')
if (await nightmare.selectHasOption('select.color', 'blue')) {
await nightmare.selectOption('select.color', 'blue')
}
if (await nightmare.selectHasOptionWithText(select.color, 'Blue color')) {
await nightmare.selectOptionWithText('select.color', 'Blue color')
} This way I won't ever clobber the global instance which I really want to avoid... |
That's really what Nightmare.action('selectHasOption', (selector, option, done) => {
return this.evaluate_now((selector, option) => {
// ...
}, done, selector, option);
});
Nightmare.action('selectOptionWithText', (selector, option, done) => {
return this.evaluate_now((selector, option) => {
// ...
}, done, selector, option);
}); Under the covers, Nightmare amends the prototype with the action you specify. |
Yes, but the problem with that I don't see a way of using that without going down a path of anti-patterns. The use case here is that I want to distribute a third party "plugin" to nightmare. The first problem with the action approach, is 1) modifying global state potentially affecting other parts of the code, and 2) require with side effects. require('nightmare-select') // Nightmare is now magically patched
const Nightmare = require('nightmare')
// ... So I refactor a bit to avoid the require with side effects. Instead of running on load, I'll export a function that will patch the nightmare object. const Nightmare = require('nightmare')
const NightmareSelect = require('nightmare-select')
NightmareSelect.patch(Nightmare)
// ... But the bigger problem still exists, I'm modifying global state. Lets say that I have several scrapers for different sites published on npm. Two of them, Now there will be a huge conflict in my app, The Node.js community is very centered on small reusable modules, and I think that publishing a lot of small modules is not an uncommon use case. Encouraging users to patch global state is dangerous and considered an anti-pattern. I would be very happy if we could see if there are any alternative approaches to a great plugin system, that is both easy to use, reliable and compatible with the Node.js ecosystem 🙌 also, Nightmare is really awesome and it beats all the other similar tools I've used, keep up the good work! |
First and foremost: I agree. Modifying the prototype directly for actions has problems. The idea behind modifying the prototype was, again, done for brevity/cleanliness and so you're not slogging through a ton of boilerplate just to get a Nightmare instance. Furthermore, patching the prototype is how internal methods are attached. Allowing users to leverage that was a reasonable outgrowth. Also, patching of Electron behavior was built atop the already-existing patching of Nightmare behavior. I'm not saying it's correct. Scope and use of Nightmare has changed. A lot. This was sort-of-kind-of touched upon in this comment. I think better plugin/action management should be put on the v3 list in #593. Thoughts? |
Merging this with #593. |
What is the intended use case for
.use(fn)
?I had hoped that it could be used to add actions to nightmare instances, but I haven't gotten that to work since it seems that I can only add actions on the global Nightmare class.
Maybe I'm just missing something...
The text was updated successfully, but these errors were encountered: