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 1 commit
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
Prev Previous commit
Next Next commit
avoiding race condition and figuring out how build/* works for cli pa…
…ckage
  • Loading branch information
JessicaSachs committed Apr 6, 2022
commit d78b3267af7eceabbda84a6572d73c0f103e56e0
3 changes: 3 additions & 0 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,9 @@ commands:
- 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: 3 additions & 3 deletions cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@
"index.mjs",
"types/**/*.d.ts",
"types/net-stubbing.ts",
"react/**",
"vue/**",
"mount-utils/**"
"mount-utils",
"vue",
"react"
],
"bin": {
"cypress": "bin/cypress"
Expand Down
16 changes: 16 additions & 0 deletions cli/scripts/start-build.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,22 @@ 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')

// 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) => {
shell.cp('-R', folder, `build/${folder}`)
})
52 changes: 33 additions & 19 deletions scripts/sync-exported-npm-with-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,55 +10,69 @@ const path = require('path')
const packlist = require('npm-packlist')
const fs = require('fs')

// 1. Get the full path to the cli where Cypress's package.json is defined
const cliPath = path.join(__dirname, '..', 'cli')
const cliPackageConfig = require(path.join(cliPath, 'package.json'))
shell.set('-v') // verbose
shell.set('-e') // any error is fatal

// 2. This script will be run in a postbuild task for each npm package
JessicaSachs marked this conversation as resolved.
Show resolved Hide resolved
// that will be re-exported by Cypress
const currentPackageDir = process.cwd()
const currentPackageConfig = require(path.join(process.cwd(), 'package.json'))

// 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 exportName = currentPackageConfig.name.replace('@cypress/', '')
const outDir = path.join(cliPath, exportName)

// 3. We'll run npm's own "packlist" to make sure we don't miss any files
// that are defined as exported in the package.json['files'] key
// 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) => {
files.forEach(async (f) => {
// 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))
}

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

// After everything is copied, let's update the Cypress cli package.json['exports'] option
// Now, we'll construct the exports map, using the module and main exports.
const isModule = cliPackageConfig.type === 'module'
// 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.indexOf(exportName) === -1) {
if (cliPackageConfig.files.includes(exportName)) {
JessicaSachs marked this conversation as resolved.
Show resolved Hide resolved
cliPackageConfig.files.push(exportName)
}

const output = JSON.stringify(cliPackageConfig, null, 2)
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.