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

Ability to define multiple 'on' events in pluginsFile without overwriting previously defined #5240

Open
Alastair-Spencer opened this issue Sep 30, 2019 · 23 comments
Labels
E2E Issue related to end-to-end testing topic: plugins ⚙️ Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: enhancement Requested enhancement of existing feature

Comments

@Alastair-Spencer
Copy link

Alastair-Spencer commented Sep 30, 2019

Current behavior:

The last declaration of the on before hook is being executed using cypress open

Desired behavior:

In this example, i'm expecting both before browser launch's to execute.

Steps to reproduce: (app code and test code)

Add following code to plugins/index.js

module.exports = (on, config) => {
  on('before:browser:launch', (browser = {}, args) => {
    console.log('WONT GET CALLED', args);
    return args;
  });

  on('before:browser:launch', (browser = {}, args) => {
    console.log('WILL GET CALLED', args);
    return args;
  });
};

Versions

v3.4.1, MacOS Mojave, Chrome / Electron

@Alastair-Spencer Alastair-Spencer changed the title Handling multiple on browser before hooks Handling multiple on browser launch hooks Sep 30, 2019
@jennifer-shehane
Copy link
Member

@Alastair-Spencer I'm a little unsure on the usecase of having 2 listeners set up for before:browser:launch, the second before:browser:launch is overwriting the initial before:browser:launch. The before:browser:launch will only be called once.

Please explain the usecase for needing 2 listeners. You may also want to utilize some of the following methods to bind or unbind from the events. https://docs.cypress.io/api/events/catalog-of-events.html#Binding-to-Events

@jennifer-shehane jennifer-shehane added the stage: needs information Not enough info to reproduce the issue label Jan 3, 2020
@Alastair-Spencer
Copy link
Author

Alastair-Spencer commented Jan 3, 2020

@jennifer-shehane

I apologise for not making the issue a little clearer - If I was to have multiple plugins hooking from the same event, this would prevent the first one from running and cause a race condition.

They look odd being placed together in this file but this is where plugins are initialised and will hook off these events being fired. Hope that helps! 👍

@liegeandlief
Copy link

liegeandlief commented Mar 24, 2020

To add to this issue, I agree that it would be useful to have multiple listeners. Currently if a plugin I use implements the before:browser:launch hook then it means I cannot also use this hook.

An example is when using one of Cypress' recommended visual regression plugins https://github.com/meinaart/cypress-plugin-snapshots.

The plugin has to be set up as follows:

const { initPlugin } = require('cypress-plugin-snapshots/plugin');

module.exports = (on, config) => {
  initPlugin(on, config);
  return config;
};

In initPlugin the before:browser:launch hook is used.

But in my plugins/index.js file I also want to use this hook in order to set the browser window size argument. So I tried this:

const { initPlugin } = require('cypress-plugin-snapshots/plugin')

module.exports = (on, config) => {
  on('before:browser:launch', (browser = {}, launchOptions) => {
    if (browser.name === 'chrome' || browser.name === 'chromium' || browser.name === 'canary') {
      launchOptions.args.push('--window-size=1200,800')
    }
    return launchOptions
  })
  
  initPlugin(on, config)
  return config
}

But only the last listener gets called. I have seen this method of plugin initialisation across several visual regression plugins suggesting this is a common way of getting plugin consumers to use the plugin, but it means that any hooks the plugin uses can't then be used by other plugins.

@jennifer-shehane jennifer-shehane changed the title Handling multiple on browser launch hooks Ability to define multiple 'on' events in pluginsFile without overwriting previously defined Jul 14, 2020
@cypress-bot cypress-bot bot added stage: proposal 💡 No work has been done of this issue and removed stage: needs information Not enough info to reproduce the issue labels Jul 14, 2020
@jennifer-shehane jennifer-shehane added topic: plugins ⚙️ type: enhancement Requested enhancement of existing feature labels Jul 14, 2020
@cooleiwhistles
Copy link

cooleiwhistles commented Feb 10, 2021

We are facing the same issue. We are using the plugin cypress-browser-permissions which modifies the before:browser:launch hook, overwriting the change in our plugins/index.js file to modify window size in before:browser:launch.

@amitzur
Copy link

amitzur commented Oct 21, 2021

@jennifer-shehane we are also facing the same issue with Applitools' SDK. We rely on the before:run and after:run hooks, and therefore our users cannot listen to these events when using our tool.
Having multiple listeners per event is actually the expected behavior from this API, similar to how EventEmitter works in Node.js (it's even called the same name - on).
Is this planned to be fixed?

@btaluy
Copy link

btaluy commented Dec 6, 2021

@jennifer-shehane we are also facing the same issue with Applitools' SDK. We rely on the before:run and after:run hooks, and therefore our users cannot listen to these events when using our tool. Having multiple listeners per event is actually the expected behavior from this API, similar to how EventEmitter works in Node.js (it's even called the same name - on). Is this planned to be fixed?

This is exactly what we are currently facing. Is there any fix planned for this?

@luxalpa
Copy link

luxalpa commented Feb 22, 2022

This just cost me 4 (thankfully paid) hours. Would be nice to get this fixed.

@gallayl
Copy link

gallayl commented Feb 25, 2022

If I register a listener, I won't expect that it will affect the already registered ones. Right now it feels like an unwanted side effect that I have to work around somehow... I expected something like how NodeJS event listeners works. Also put some futile effort to figure out what's going on :)

on is a function that you will use to register listeners on various events that Cypress exposes.

from Cypress docs

@cypress-bot cypress-bot bot added stage: icebox and removed stage: proposal 💡 No work has been done of this issue labels Apr 28, 2022
@mikstime
Copy link

We are facing the same issue. Here is a cheap workaround for multiple listeners on each event.

const makeChainableListeners = () => {
    const events = {};
    const chainListeners = (action, fn) => {
        if (!events[action]) {
            events[action] = [];
        }
        events[action].push(fn);
    };
    const applyListeners = on => {
        for (const [action, fns] of Object.entries(events)) {
            if (action === 'task') {
                on(action, fns.reduce((a, v) => ({ ...a, ...v }), {}));
            } else {
                on(action, async function(...args) {
                    for (const fn of fns) {
                        await fn.apply(this, args);
                    }
                });
            }
        }
    };
    return [chainListeners, applyListeners];
};

export default async function plugins(on, config) {
    const [chainListeners, applyListeners] = makeChainableListeners();
    chainListeners('after:run', () => console.log('after run 1'));
    chainListeners('after:run', () => console.log('after run 2'));
    initPlugin(chainListeners, config);

    applyListeners(on);

    return config;
};

@zawiasam
Copy link

zawiasam commented Jul 1, 2022

We are facing the same issue. Here is a cheap workaround for multiple listeners on each event.

const makeChainableListeners = () => {
    const events = {};
    const chainListeners = (action, fn) => {
        if (!events[action]) {
            events[action] = [];
        }
        events[action].push(fn);
    };
    const applyListeners = on => {
        for (const [action, fns] of Object.entries(events)) {
            if (action === 'task') {
                on(action, fns.reduce((a, v) => ({ ...a, ...v }), {}));
            } else {
                on(action, async function(...args) {
                    for (const fn of fns) {
                        await fn.apply(this, args);
                    }
                });
            }
        }
    };
    return [chainListeners, applyListeners];
};

export default async function plugins(on, config) {
    const [chainListeners, applyListeners] = makeChainableListeners();
    chainListeners('after:run', () => console.log('after run 1'));
    chainListeners('after:run', () => console.log('after run 2'));
    initPlugin(chainListeners, config);

    applyListeners(on);

    return config;
};

Just keep in mind that this will not work in case when you need to return value like in case of "before:browser:launch"
You will have to adjust this code for your needs

derevnjuk added a commit to NeuraLegion/cypress-har-generator that referenced this issue Nov 9, 2022
Function has been deprecated. Use the `install` function instead as follows:
```diff
setupNodeEvents(on) {
  install(on);
-  // bind to the event we care about
-  on('before:browser:launch', (browser = {}, launchOptions) => {
-    ensureBrowserFlags(browser, launchOptions);
-    return launchOptions;
-  });
}
```
In case of any issues please refer to cypress-io/cypress#5240
derevnjuk added a commit to NeuraLegion/cypress-har-generator that referenced this issue Nov 9, 2022
The `ensureBrowserFlags` function has been deprecated. Use the `install` function instead as follows:

```diff
setupNodeEvents(on) {
  install(on);
-
-  on('before:browser:launch', (browser = {}, launchOptions) => {
-    ensureBrowserFlags(browser, launchOptions);
-    return launchOptions;
-  });
}
```
In case of any issues please refer to cypress-io/cypress#5240

closes #100
@brian-mann
Copy link
Member

brian-mann commented Dec 28, 2022

Yeah we should definitely allow certain (or all) event handlers to be added, but we'd need to define the specification for this behavior. For instance, should the callbacks be called simultaneously, or async sequentially, yielding modified objects in the callback function, etc.

We'd also need to factor in how you potentially "remove" a listener, since we only yield you the on object, effectively we'd be inheriting all of the methods of how event-emitter works.

Once we define the specification and the expected behavior (this would also likely be a breaking change) then we could go ahead and implementation. Definitely in favor of doing this though.

@emilyrohrbough
Copy link
Member

related to #22428

@francisco-polaco
Copy link

This just made me spend 8 hours 😢
We're using cypress-plugin-snapshots and my before:browser:launch listener wasn't being called.

I believe a lot of people will and already bumped into this.

@ext
Copy link

ext commented Mar 9, 2023

A workaround we hacked together is to wrap the on function with an EventEmitter forwarding all events:

class EventForwarder {
    private emitter: EventEmitter;
    private task: Cypress.Tasks;
    public on: Cypress.PluginEvents;

    public constructor() {
        this.emitter = new EventEmitter();
        this.task = {};
        this.on = (action, arg) => {
            if (action === "task") {
                Object.assign(this.task, arg);
            } else {
                this.emitter.on(action, arg as () => void);
            }
        };
    }

    public forward(on: Cypress.PluginEvents): void {
        for (const event of this.emitter.eventNames()) {
            /* eslint-disable-next-line @typescript-eslint/no-explicit-any */
            on(event as any, (...args: unknown[]) => {
                for (const listener of this.emitter.listeners(event)) {
                    listener(...args);
                }
            });
        }
        on("task", this.task);
    }
}

It can be used as following:

 export default defineConfig({
     e2e: {
-        setupNodeEvents(on, config) {
+        setupNodeEvents(cypressOn, config) {
+            const eventForwarder = new EventForwarder();
+            const on = eventForwarder.on;
             
             plugin1(on);
             plugin2(on);
             plugin3(on);
 
             on("before:run", () => {
                 /* ... */
             });
+
+            eventForwarder.forward(cypressOn);
        },
    }
});

@Devstored
Copy link

Devstored commented Mar 24, 2023

Hi @ext

Can you post an example code please? In my case I use the library: cypress-aiotests-reporter + cypress-mochawesome-reporter.

In the file "cypress.config.js" :

const { EventForwarder } = require("./eventForwarder");
e2e: {
    //setupNodeEvents(on, config) {
    setupNodeEvents(cypressOn, config) {
      const eventForwarder = new EventForwarder();
      const on = eventForwarder.on;
      registerAIOTestsPlugin(on,config);
      require('@cypress/grep/src/plugin')(config)
      require('cypress-mochawesome-reporter/plugin')(on)
      on('before:run', async (details) => {
        console.log('override before:run');
        await beforeRunHook(details);
      });
      
      on('after:run', async () => {
        console.log('override after:run');
        await afterRunHook();
      });
      eventForwarder.forward(cypressOn);
      on('task', {
        getFiles(path) {
          let list = [];
          return getFiles(path, list)
        },
        log (message) {
          console.log(message)
          return null
        },
        
      
      
      return config
    },
  },

I created an "event Forwarder.ts" file at the root of the project:

// eventForwarder.ts
import { EventEmitter } from "events";

export class EventForwarder {
    private emitter: EventEmitter;
    private task: Cypress.Tasks;
    public on: Cypress.PluginEvents;

    public constructor() {
        this.emitter = new EventEmitter();
        this.task = {};
        this.on = (action, arg) => {
            if (action === "task") {
                Object.assign(this.task, arg);
            } else {
                this.emitter.on(action, arg as () => void);
            }
        };
    }

    public forward(on: Cypress.PluginEvents): void {
        for (const event of this.emitter.eventNames()) {
            /* eslint-disable-next-line @typescript-eslint/no-explicit-any */
            on(event as any, (...args: unknown[]) => {
                for (const listener of this.emitter.listeners(event)) {
                    listener(...args);
                }
            });
        }
        on("task", this.task);
    }
}

I created a "tsconfig.json" file at the root of the project:

    "compilerOptions": {
      "target": "es5",
      "lib": [
        "es5",
        "dom"
      ],
      "types": [
        "node",
        "cypress",
        "cypress-mochawesome-reporter"
      ]
    },
    "include": [
      "**/*.ts"
    ]
  }

The plugins run fine but the content of the .html file for cypress-mochawesome-reporter is empty. I do not understand why ?

Thanks for your help !

@elaichenkov
Copy link

Hi all,
I've faced the same issue. So, I decided to create a library that can fix the problem. You just need to install the library with the following command:

npm i -D cypress-plugin-init 

And then, you need to import the initPlugins function and use it in your cypress.config.ts file:

import { initPlugins } from 'cypress-plugin-init';

export default defineConfig({
  e2e: {
   // ...
    setupNodeEvents(on, config) {
      // invoke the function with all plugins that you need instead of the 'plugin1' and 'plugin2'
      initPlugins(on, [plugin1, plugin2]);
    },
   // ...
  },
});

@Devstored
Copy link

Devstored commented Mar 29, 2023

Hi @elaichenkov ,

Thanks for your sharing but not work for me :

initPlugins(on, [registerAIOTestsPlugin(on,config), require('cypress-mochawesome-reporter/plugin')(on)]);

Traceback :


TypeError: plugin is not a function
    at /Users/dino/Documents/tests/node_modules/cypress-plugin-init/index.js:16:9
    at Array.forEach (<anonymous>)
    at initPlugins (/Users/dino/Documents/tests/node_modules/cypress-plugin-init/index.js:15:13)
    at setupNodeEvents (/Users/dino/Documents/tests/cypress.config.js:84:7)
    at /Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:122:14
    at tryCatcher (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/node_modules/bluebird/js/release/util.js:16:23)
    at Function.Promise.attempt.Promise.try (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/node_modules/bluebird/js/release/method.js:39:29)
    at RunPlugins.load (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:119:9)
    at RunPlugins.runSetupNodeEvents (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:59:17)
    at EventEmitter.<anonymous> (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:185:22)
    at EventEmitter.emit (node:events:513:28)
    at EventEmitter.emit (node:domain:489:12)
    at process.<anonymous> (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/util.js:33:22)
    at process.emit (node:events:513:28)
    at process.emit (node:domain:489:12)
    at process.emit.sharedData.processEmitHook.installedValue [as emit] (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/node_modules/@cspotcode/source-map-support/source-map-support.js:745:40)
    at emit (node:internal/child_process:937:14)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.```

@elaichenkov
Copy link

elaichenkov commented Mar 29, 2023

Hi @elaichenkov ,

Thanks for your sharing but not work for me :

initPlugins(on, [registerAIOTestsPlugin(on,config), require('cypress-mochawesome-reporter/plugin')(on)]);

Traceback :


TypeError: plugin is not a function
    at /Users/dino/Documents/tests/node_modules/cypress-plugin-init/index.js:16:9
    at Array.forEach (<anonymous>)
    at initPlugins (/Users/dino/Documents/tests/node_modules/cypress-plugin-init/index.js:15:13)
    at setupNodeEvents (/Users/dino/Documents/tests/cypress.config.js:84:7)
    at /Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:122:14
    at tryCatcher (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/node_modules/bluebird/js/release/util.js:16:23)
    at Function.Promise.attempt.Promise.try (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/node_modules/bluebird/js/release/method.js:39:29)
    at RunPlugins.load (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:119:9)
    at RunPlugins.runSetupNodeEvents (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:59:17)
    at EventEmitter.<anonymous> (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_require_async_child.js:185:22)
    at EventEmitter.emit (node:events:513:28)
    at EventEmitter.emit (node:domain:489:12)
    at process.<anonymous> (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/util.js:33:22)
    at process.emit (node:events:513:28)
    at process.emit (node:domain:489:12)
    at process.emit.sharedData.processEmitHook.installedValue [as emit] (/Users/dino/Library/Caches/Cypress/12.8.1/Cypress.app/Contents/Resources/app/node_modules/@cspotcode/source-map-support/source-map-support.js:745:40)
    at emit (node:internal/child_process:937:14)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.```

Hey @Devstored
You don't have to invoke the plugins. Just pass them in the array:

- initPlugins(on, [registerAIOTestsPlugin(on,config), require('cypress-mochawesome-reporter/plugin')(on)]);
+ initPlugins(on, [registerAIOTestsPlugin, require('cypress-mochawesome-reporter/plugin')]);

However, I think that won't work as well. Because, currently, the plugin accepts only one parameter (on). But in your case, registerAIOTestsPlugin expects two parameters. Please, create an issue in the repo if it doesn't work.

@Devstored
Copy link

Devstored commented Mar 29, 2023

@elaichenkov

Thanks, but i tried and same issue :
`An error was thrown in your plugins file while executing the handler for the before:run event.

The error we received was:

TypeError: Cannot convert undefined or null to object`

@elaichenkov
Copy link

@Devstored
Please, update the plugin to the 0.0.7 version and try again:

import { initPlugins } from 'cypress-plugin-init';

export default defineConfig({
  e2e: {
    // ...
    setupNodeEvents(on, config) {
      initPlugins(on, [registerAIOTestsPlugin, require('cypress-mochawesome-reporter/plugin')], config);
    },
    // ...
  },
});

@Devstored
Copy link

Hi @elaichenkov,
It works perfectly!

Thanks for your help

@skitterm
Copy link

I wanted to add my use-case as a plugin developer:

My Cypress plugin makes use of multiple events (before:run and task). Currently, I either have to have an install command (that will override or be overridden by any events of the same name in the users' setupNodeEvents) or the user has to import and individually call my functions for each event, which is cumbersome for them.

Just wanted to advocate for merging events automatically in Cypress, otherwise users with multiple plugins or custom events of their own will be out of luck.

@mistic100
Copy link
Contributor

I don't understand why this is still not fixed. A lot of reporting plugins hook to the 'after:run', 'after:spec' events. While Gleb Bahmutov's cypress-on-fix package works perfectly, it should not be necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2E Issue related to end-to-end testing topic: plugins ⚙️ Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: enhancement Requested enhancement of existing feature
Projects
None yet
Development

No branches or pull requests