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

feat: embedding mount into the cypress binary (real dependency) #20930

Merged
merged 16 commits into from
Apr 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ system-tests/projects/config-with-ts-syntax-error/**
# cli/types is linted by tslint/dtslint
cli/types

# cli/react, cli/vue, and cli/mount-utils are all copied from dist'd builds
cli/react
cli/vue
cli/mount-utils

# packages/example is not linted (think about changing this)
packages/example

Expand Down
7 changes: 7 additions & 0 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -948,9 +948,16 @@ commands:
- run:
name: Build NPM package
command: yarn build --scope cypress
- run:
name: Copy Re-exported NPM Packages
command: node ./scripts/post-build.js
working_directory: cli
- run:
command: ls -la types
working_directory: cli/build
- run:
command: ls -la vue mount-utils react
working_directory: cli/build
- unless:
condition:
equal: [ *windows-executor, << parameters.executor >> ]
Expand Down
6 changes: 6 additions & 0 deletions cli/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,9 @@ types/sinon-chai
types/net-stubbing.ts
# ignore CLI output build folder
build

# ignore packages synced at build-time via
# the sync-exported-npm-with-cli.js script
vue
react
mount-utils
41 changes: 41 additions & 0 deletions cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,47 @@ yarn add ~/{your-dirs}/cypress/cli/build/cypress-3.3.1.tgz --ignore-scripts

Which installs the `tgz` file we have just built from folder `Users/jane-lane/{your-dirs}/cypress/cli/build`.

#### Sub-package API

> How do deep imports from cypress/* get resolved?

The cypress npm package comes pre-assembled with mounting libraries for major front-end frameworks. These mounting libraries are the first examples of Cypress providing re-exported sub-packages. These sub-packages follow the same naming convention they do when they're published on **npm**, but without a leading **`@`** sign. For example:

##### An example of a sub-package: @cypress/vue, @cypress/react, @cypress/mount-utils

**Let's discuss the Vue mounting library that Cypress ships.**

If you'd installed the `@cypress/vue` package from NPM, you could write the following code.

This would be necessary when trying to use a version of Vue, React, or other library that may be newer or older than the current version of cypress itself.

```js
import { mount } from '@cypress/vue'
```

Now, with the sub-package API, you're able to import the latest APIs directly from Cypress without needing to install a separate dependency.

```js
import { mount } from 'cypress/vue'
```

The only difference is the import name, and if you still need to use a specific version of one of our external sub-packages, you may install it and import it directly.

##### Adding a new sub-package

There are a few steps when adding a new sub-package.

1. Make sure the sub-package's rollup build is _self-contained_ or that any dependencies are also declared in the CLI's **`package.json`**.
2. Now, in the **`postbuild`** script for the sub-package you'd like to embed, invoke `node ./scripts/sync-exported-npm-with-cli.js` (relative to the sub-package, see **`npm/vue`** for an example).
3. Add the sub-package's name to the following locations:
- **`cli/.gitignore`**
- **`cli/scripts/post-build.js`**
- **`.eslintignore`** (under cli/sub-package)
4. DO NOT manually update the **package.json** file. Running `yarn build` will automate this process.
5. Commit the changed files.

[Here is an example Pull Request](https://github.com/cypress-io/cypress/pull/20930/files#diff-21b1fe66043572c76c549a4fc5f186e9a69c330b186fc91116b9b70a4d047902)

#### Module API

The module API can be tested locally using something like:
Expand Down
16 changes: 15 additions & 1 deletion cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@
"index.js",
"index.mjs",
"types/**/*.d.ts",
"types/net-stubbing.ts"
"types/net-stubbing.ts",
"mount-utils",
"vue",
"react"
],
"bin": {
"cypress": "bin/cypress"
Expand All @@ -117,9 +120,20 @@
"import": "./index.mjs",
"require": "./index.js"
},
"./vue": {
"import": "./vue/dist/cypress-vue.esm-bundler.js",
"require": "./vue/dist/cypress-vue.cjs.js"
},
"./package.json": {
"import": "./package.json",
"require": "./package.json"
},
"./react": {
"import": "./react/dist/cypress-react.esm-bundler.js",
"require": "./react/dist/cypress-react.cjs.js"
},
"./mount-utils": {
"require": "./mount-utils/dist/index.js"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No import entry for mount-utils?

Copy link
Contributor Author

@JessicaSachs JessicaSachs Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it wasn't built for ESM.

Because only cypress-react and cypress-vue consume it and they have an esm export it's not (currently) a problem.

We have tech debt to convert all of our npm/* packages to be esm-first and share the same rollup + typescript compilation settings.

}
}
}
21 changes: 21 additions & 0 deletions cli/scripts/post-build.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const shell = require('shelljs')
const { resolve } = require('path')

shell.set('-v') // verbose
shell.set('-e') // any error is fatal

// For each npm package that is re-published via cypress/*
// make sure that it is also copied into the build directory
const npmModulesToCopy = [
'mount-utils',
'react',
'vue',
]

npmModulesToCopy.forEach((folder) => {
// cli/mount-utils => cli/build/mount-utils
const from = resolve(`${__dirname}/../${folder}`)
const to = resolve(`${__dirname}/../build/${folder}`)

shell.cp('-R', from, to)
})
4 changes: 4 additions & 0 deletions cli/scripts/start-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ includeTypes.forEach((folder) => {
shell.cp('-R', source, 'build/types')
})

// TODO: Add a typescript or rollup build step
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a github issue for this we can link? Have been trying to do TODO: blah, https://github.com/blahblah.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't right now.

// The only reason start-build.js exists
// is because the cli package does not have an actual
// build process to compile index.js and lib
shell.exec('babel lib -d build/lib')
shell.exec('babel index.js -o build/index.js')
shell.cp('index.mjs', 'build/index.mjs')
3 changes: 2 additions & 1 deletion npm/mount-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"main": "dist/index.js",
"scripts": {
"build": "tsc || echo 'built, with type errors'",
"build-prod": "tsc || echo 'built, with type errors'",
"postbuild": "node ../../scripts/sync-exported-npm-with-cli.js",
"build-prod": "yarn build",
"check-ts": "tsc --noEmit",
"watch": "tsc -w"
},
Expand Down
1 change: 1 addition & 0 deletions npm/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"main": "dist/cypress-react.cjs.js",
"scripts": {
"build": "rimraf dist && yarn transpile-plugins && rollup -c rollup.config.js",
"postbuild": "node ../../scripts/sync-exported-npm-with-cli.js",
"build-prod": "yarn build",
"cy:open": "node ../../scripts/cypress.js open --component",
"cy:open:debug": "node --inspect-brk ../../scripts/start.js --component-testing --run-project ${PWD}",
Expand Down
1 change: 0 additions & 1 deletion npm/react/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ function createEntry (options) {
external: [
'react',
'react-dom',
'@cypress/mount-utils',
],
plugins: [
resolve(), commonjs(),
Expand Down
1 change: 1 addition & 0 deletions npm/vue/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"main": "dist/cypress-vue.cjs.js",
"scripts": {
"build": "rimraf dist && rollup -c rollup.config.js",
"postbuild": "node ../../scripts/sync-exported-npm-with-cli.js",
"build-prod": "yarn build",
"cy:open": "node ../../scripts/cypress.js open --component --project ${PWD}",
"cy:run": "node ../../scripts/cypress.js run --component --project ${PWD}",
Expand Down
4 changes: 0 additions & 4 deletions npm/vue/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ function createEntry (options) {
input,
external: [
'vue',
'@vue/test-utils',
'@cypress/mount-utils',
'@cypress/webpack-dev-server',
],
plugins: [
resolve({ preferBuiltins: true }), commonjs(),
Expand All @@ -36,7 +33,6 @@ function createEntry (options) {
format,
globals: {
vue: 'Vue',
'@vue/test-utils': 'VueTestUtils',
},
exports: 'auto',
},
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
"binary-release": "node ./scripts/binary.js release",
"binary-upload": "node ./scripts/binary.js upload",
"binary-zip": "node ./scripts/binary.js zip",
"build": "lerna run build --stream --no-bail --ignore create-cypress-tests --ignore \"'@packages/{runner}'\" && lerna run build --stream --scope create-cypress-tests",
"build-prod": "lerna run build-prod-ui --stream && lerna run build-prod --stream --ignore create-cypress-tests && lerna run build-prod --stream --scope create-cypress-tests",
"build": "lerna run build --stream --no-bail --ignore create-cypress-tests --ignore \"'@packages/{runner}'\" && node ./cli/scripts/post-build.js && lerna run build --stream --scope create-cypress-tests",
"build-prod": "lerna run build-prod-ui --stream && lerna run build-prod --stream --ignore create-cypress-tests && node ./cli/scripts/post-build.js && lerna run build-prod --stream --scope create-cypress-tests",
"check-node-version": "node scripts/check-node-version.js",
"check-terminal": "node scripts/check-terminal.js",
"clean": "lerna run clean --parallel --no-bail || echo 'ok, errors while cleaning'",
Expand Down
78 changes: 78 additions & 0 deletions scripts/sync-exported-npm-with-cli.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/**
* This script is used to re-export packages that Cypress publishes on its own.
* It is executed individually in a postbuild step by individual npm/* packages.
* For example, usually, Cypress will publish the `npm/react` directory as a `@cypress/react` package.
* However, the Cypress binary will also ship an export for `cypress/react` that's guaranteed to work
* with this version of the binary
*/
const shell = require('shelljs')
const path = require('path')
const packlist = require('npm-packlist')
const fs = require('fs')

shell.set('-v') // verbose
shell.set('-e') // any error is fatal

// This script will be run in a postbuild task for each npm package
// that will be re-exported by Cypress
const currentPackageDir = process.cwd()

// 1. We'll run npm's own "packlist" against the npm package to be published (@cypress/react, etc)
// to make sure we don't miss any files when we copy them over to the CLI package
// The files that will be returned here are the ones from @cypress/react's package.json['files'] key.
packlist({ path: currentPackageDir })
.then((files) => {
// 2. Move all of the files that would be published under @cypress/react
// to be copied under cli/react (drop the @cypress namespace)
const cliPath = path.join(__dirname, '..', 'cli')

// Typically, these packages are independently published as @cypress/package-name
// e.g. @cypress/vue => import whatever from 'cypress/vue'
// The files will wind up at cypress/cli/vue/*
const currentPackageConfig = require(path.join(process.cwd(), 'package.json'))
const exportName = currentPackageConfig.name.replace('@cypress/', '')
const outDir = path.join(cliPath, exportName)

// 3. For each file, mkdir if not exists, and then copy the dist'd assets over
// Shell is synchronous by default, but we don't actually need to await for the results
// to write to the `cliPackageConfig` at the end
files.forEach((f) => {
// mkdir if not exists
const { dir } = path.parse(f)

if (dir) {
shell.mkdir('-p', path.join(outDir, dir))
}

shell.cp(path.join(currentPackageDir, f), path.join(outDir, f))
})

// After everything is copied, let's update the Cypress cli package.json['exports'] map.
const isModule = currentPackageConfig.type === 'module'

const cliPackageConfig = require(path.join(cliPath, 'package.json'))

const subPackageExports = cliPackageConfig.exports[`./${exportName}`] = {}
const esmEntry = isModule ? currentPackageConfig.main : currentPackageConfig.module

if (esmEntry) {
// ./react/dist/cypress-react-esm.js, etc
subPackageExports.import = `./${exportName}/${esmEntry}`
}

if (!isModule) {
// ./react/dist/cypress-react-cjs.js, etc
subPackageExports.require = `./${exportName}/${currentPackageConfig.main}`
}

if (!cliPackageConfig.files.includes(exportName)) {
cliPackageConfig.files.push(exportName)
}

const output = `${JSON.stringify(cliPackageConfig, null, 2) }\n`

// eslint-disable-next-line no-console
console.log('Writing to CLI package.json for', exportName)

fs.writeFileSync(path.join(cliPath, 'package.json'), output, 'utf-8')
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tiniest thing but this seems like pretty important logic to test - do we need some kind of basic test to ensure that the package.json updates correctly, and that import { mount } from 'cypress/vue is actually working? We don't have any coverage for this right now, from what I can see.

I wonder if we can update all internal code to use the deep import syntax in the future. Is this something we plan to do?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can test it by adding code here cypress-io/cypress-example-kitchensink#528

I'll do that. It'll break all other pipelines until this is merged, maybe I'll create a new branch for it (in kitchensink).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also wondering if we can convert a couple of the existing system tests to import { mount } from 'cypress/vue' rather than import { mount } from '@cypress/vue'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That probably only works in test-binary, where we have tests that run in a docker container against the compiled binary. But still worth looking into. Just an extremely simple 'can import' test there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah in general I feel uncomfortable that we don't have a lot of post-build infrastructure for testing the output of Circle CI's build scripts. The kitchen sink is the right place. I put a PR up above that will at least fail if those packages aren't declared in the exports map.