refactor: make the network shim cypress-plugin based

[HendrikThePendric]

Before this PR, the networkShim executed some standalone code to clear the fixtures folder. This PR refactors the NetworkShim to be a combination of a setup and a plugin and the standalone code has been removed. The brings several advantages:

1. Albeit in a slightly different way, both the setup and the plugin are now reading properties from the resolved cypress config. This will prevent discrepancies.
2. In more general terms, the network shim is now 100% built in / on top of cypress, which seems sensible.
3. The NetworkShim used to assume the fixture files would always be in /cypress/fixtures/network but the fixtures destination is actually configurable. Because we can access the fixtures directory path from the cypress config we don't need to make assumptions about this anymore, so this solves a known limitation.

As discussed with @amcgee, this plugin based solution still has one known limitation, which is that capturing doesn't work when the run is being done in parallel, because each (parallel) run gets its own plugin instance. The current solution will throw an error if it detects a parallel run in capture mode. I have created an issue that documents the challenges and suggests some possible solutions if we do need to support parallel runs in the future.

Timing-wise I don't think we should this merge this in right now as it will produce a new major version. I think this #115 will also produce breaking changes, so I think we should introduce both of these PRs into master together.

When we bumped from 4 to 5 we included a few codemods. I have not included any codemods in this PR, but if this is what we want to do, I guess I could add a codemode that imports networkShim into the plugins index file and calls networkShim(on). Personally I think that just writing a good changelog and/or migration guide is enough. I'm not a big fan of all this magical codemodding.

This PR also includes some not directly related changes: because cypress plugins are NodeJS modules we don't really need to transpile them. Instead we can just use the source code directly. So I've changed some of the code in the plugins modules a little bit and I've removed the build script for the plugins package as well.

BREAKING CHANGE: To use the network-shim's functionality the project will need to use the new network shim plugin on top of the existing enableNetworkShim setup. Also, the network shim options (an object with hosts and staticResources) need to be passed to the networkShim plugin, the enableNetworkShim setup doesn't accept any arguments anymore.

[varl]

Timing-wise I don't think we should this merge this in right now as it will produce a new major version. I think this #115 will also produce breaking changes, so I think we should introduce both of these PRs into master together.

As discussed on Slack this isn't a blocker now. 😅

When we bumped from 4 to 5 we included a few codemods. I have not included any codemods in this PR, but if this is what we want to do, I guess I could add a codemode that imports networkShim into the plugins index file and calls networkShim(on). Personally I think that just writing a good changelog and/or migration guide is enough. I'm not a big fan of all this magical codemodding.

Codemods are quite robust and will make it a lot easier to help all apps get up to new versions so I definitely think that if we can supply a codemod to apply the breaking change automatically, we should do so.

[varl]

As discussed with @amcgee, this plugin based solution still has one known limitation, which is that capturing doesn't work when the run is being done in parallel, because each (parallel) run gets its own plugin instance. The current solution will throw an error if it detects a parallel run in capture mode. I have created an issue that documents the challenges and suggests some possible solutions if we do need to support parallel runs in the future.

Nice solution on throwing if a parallel run is detected. I think the limitation is OK for now. 👍 Good job.

[HendrikThePendric]

@amcgee @Mohammer5 @varl there are some points I am unsure about:
POINT 1:
Triggering a major version release: I'm mentioning the word BREAKING CHANGE both in 760ba42 and in the PR description. I think/hope that means either a merge-commit or a squash-and-merge would trigger a major version bump. Please confirm.

POINT 2:
Also, apps will need to make a few changes (hence the breaking change):

• In /cypress/plugins/index.js they will need to call the plugin with the options arg, for example: networkShim(on, { staticResources: ['animals'] })
• In /cypress/commands/index.js they will need to still call enableNetworkShim(). However, now without the options arg.

These changes are pretty much undocumented (the only source of information is the example app). Should I take additional steps to make the transition easier?

[HendrikThePendric]

Codemods are quite robust and will make it a lot easier to help all apps get up to new versions so I definitely think that if we can supply a codemod to apply the breaking change automatically, we should do so.

Missed that. I will see what I can do.

[HendrikThePendric]

Missed that. I will see what I can do.

@varl in 759dc44 I've made some adjustments in the install command that will make sure the new plugin is added correctly. I've also included some changes that would theoretically do the same for the enableNetworkShim setup (commands package). However, when testing this out in the example app, I noticed that no changes to ./cypress/commands/index.js were being produced. I'm pretty sure this is a pre-existing bug, otherwise I would have expected to see some unrelated changes (i.e. import and call of enableAutoLogin) to have happened at least.

So what do we do now? I guess these are our options:
A) Fix the install command here?
B) Keep things as they are fix things in another branch?

I'd be in favour of B, but that could be because it's the least amount of work right now, and that shouldn't drive our decisions....

[varl]

@amcgee @Mohammer5 @varl there are some points I am unsure about:
POINT 1:
Triggering a major version release: I'm mentioning the word BREAKING CHANGE both in 760ba42 and in the PR description. I think/hope that means either a merge-commit or a squash-and-merge would trigger a major version bump. Please confirm.

If you squash-merge, you must make sure that the final commit message is conventional compliant, the default one used by GitHub is usually not.

If you merge, then the commit message is going to trigger a breaking change.

POINT 2:
Also, apps will need to make a few changes (hence the breaking change):

* In `/cypress/plugins/index.js` they will need to call the plugin with the options arg, for example: `networkShim(on, { staticResources: ['animals'] })`

* In `/cypress/commands/index.js` they will need to still call `enableNetworkShim()`. However, now without the options arg.

These changes are pretty much undocumented (the only source of information is the example app). Should I take additional steps to make the transition easier?

Some documentation would be nice, at the very least in README under an "Migrating from X to Y?" section with the required steps.

[varl]

In general, the commit that triggers the change should be structured:

feat: short summary of the feature

A longer description of the change.

BREAKING CHANGE: description of the breaking change and what steps are required to resolve it.
Possible to have multiple lines describing a breaking change.

BREAKING CHANGE: second breaking change that requires the following steps to resolve it.
Do x, y, and z but not å to resolve the breaking change in your app.

That way, each breaking change shows up in the CHANGELOG, with the steps the consumer needs to take to comply with the breaking changes.

[HendrikThePendric]

Thanks for the info @varl I'll go with a merge commit and am assured 760ba42 will trigger the major version bump.

[dhis2-bot]

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀