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: server logic for create from React component #24881

Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
6c24bfa
feat: WIP server logic for create from React component
astone123 Nov 29, 2022
2948dd7
feat: add more tests; error handling
astone123 Nov 29, 2022
73cb71f
feat: PR feedback [run CI]
astone123 Dec 1, 2022
0dbd568
Resolve conflicts
astone123 Dec 1, 2022
0f4e836
feat: try committing snapshot cache changes [run ci]
astone123 Dec 1, 2022
d763e1f
feat: try re-generating snapshot [run ci]
astone123 Dec 1, 2022
a2626df
fix build
ryanthemanuel Dec 1, 2022
6eabeb3
regenerate cache on darwin
ryanthemanuel Dec 1, 2022
6ba1695
update caches
ryanthemanuel Dec 1, 2022
9389037
Revert "feat: try re-generating snapshot [run ci]"
astone123 Dec 1, 2022
d4e4c7d
Merge branch 'feature/create-from-react-component' into astone123/cre…
lmiller1990 Dec 2, 2022
a48491d
fix typing error
lmiller1990 Dec 2, 2022
00e7819
types
lmiller1990 Dec 2, 2022
df027ff
fix test
lmiller1990 Dec 2, 2022
b32560f
chore: try using [email protected]
lmiller1990 Dec 2, 2022
89b823a
update test
lmiller1990 Dec 2, 2022
2f3ffcb
regen linux snapshot
lmiller1990 Dec 2, 2022
a631284
update snapshots for darwin
lmiller1990 Dec 2, 2022
dbb5b3e
re-gen linux snapshot
lmiller1990 Dec 2, 2022
4ad474c
Merge branch 'astone123/create-from-react-server' of https://github.c…
lmiller1990 Dec 2, 2022
c0382fa
yarn install
lmiller1990 Dec 2, 2022
7456bb7
update snapshots
lmiller1990 Dec 2, 2022
fadb8d9
update snapshot metadata
lmiller1990 Dec 2, 2022
6d502b1
update snapshots due to babel deps changing slightly
lmiller1990 Dec 2, 2022
f60ac0a
make react docgen a dep
lmiller1990 Dec 2, 2022
bd398ed
update tests
lmiller1990 Dec 2, 2022
9510a85
revert
lmiller1990 Dec 2, 2022
77a4942
snapshots again??
lmiller1990 Dec 2, 2022
f9e8af7
revert
lmiller1990 Dec 2, 2022
68d5c87
update
lmiller1990 Dec 2, 2022
281da0a
update
lmiller1990 Dec 2, 2022
e4410b1
try change snapshot
lmiller1990 Dec 2, 2022
f12efca
change snap
lmiller1990 Dec 2, 2022
e857b8f
update snap
lmiller1990 Dec 2, 2022
c070f96
feat: remove unnecessary ts-ignore
astone123 Dec 2, 2022
cd65233
feat: add more test cases
astone123 Dec 2, 2022
9449d66
feat: create CodegenActions; other minor refactors
astone123 Dec 5, 2022
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
2 changes: 1 addition & 1 deletion npm/webpack-preprocessor/__snapshots__/compilation.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
exports['webpack preprocessor - e2e correctly preprocesses the file 1'] = `
it("is a test",(function(){expect(1).to.equal(1),expect(2).to.equal(2),expect(Math.min.apply(Math,[3,4])).to.equal(3)}));
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZXhhbXBsZV9zcGVjX291dHB1dC5qcyIsIm1hcHBpbmdzIjoiQUFBQUEsR0FBRyxhQUFhLFdBR2RDLE9BRmdCLEdBRU5DLEdBQUdDLE1BQU0sR0FDbkJGLE9BSG1CLEdBR1RDLEdBQUdDLE1BQU0sR0FDbkJGLE9BQU9HLEtBQUtDLElBQUwsTUFBQUQsS0FBWSxDQUFDLEVBQUcsS0FBS0YsR0FBR0MsTUFBTSIsInNvdXJjZXMiOlsid2VicGFjazovL0BjeXByZXNzL3dlYnBhY2stcHJlcHJvY2Vzc29yLy4vdGVzdC9fdGVzdC1vdXRwdXQvZXhhbXBsZV9zcGVjLmpzIl0sInNvdXJjZXNDb250ZW50IjpbIml0KCdpcyBhIHRlc3QnLCAoKSA9PiB7XG4gIGNvbnN0IFthLCBiXSA9IFsxLCAyXVxuXG4gIGV4cGVjdChhKS50by5lcXVhbCgxKVxuICBleHBlY3QoYikudG8uZXF1YWwoMilcbiAgZXhwZWN0KE1hdGgubWluKC4uLlszLCA0XSkpLnRvLmVxdWFsKDMpXG59KVxuIl0sIm5hbWVzIjpbIml0IiwiZXhwZWN0IiwidG8iLCJlcXVhbCIsIk1hdGgiLCJtaW4iXSwic291cmNlUm9vdCI6IiJ9
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiZXhhbXBsZV9zcGVjX291dHB1dC5qcyIsIm1hcHBpbmdzIjoiQUFBQUEsR0FBRyxhQUFhLFdBR2RDLE9BRmdCLEdBRU5DLEdBQUdDLE1BQU0sR0FDbkJGLE9BSG1CLEdBR1RDLEdBQUdDLE1BQU0sR0FDbkJGLE9BQU9HLEtBQUtDLElBQUcsTUFBUkQsS0FBWSxDQUFDLEVBQUcsS0FBS0YsR0FBR0MsTUFBTSIsInNvdXJjZXMiOlsid2VicGFjazovL0BjeXByZXNzL3dlYnBhY2stcHJlcHJvY2Vzc29yLy4vdGVzdC9fdGVzdC1vdXRwdXQvZXhhbXBsZV9zcGVjLmpzIl0sInNvdXJjZXNDb250ZW50IjpbIml0KCdpcyBhIHRlc3QnLCAoKSA9PiB7XG4gIGNvbnN0IFthLCBiXSA9IFsxLCAyXVxuXG4gIGV4cGVjdChhKS50by5lcXVhbCgxKVxuICBleHBlY3QoYikudG8uZXF1YWwoMilcbiAgZXhwZWN0KE1hdGgubWluKC4uLlszLCA0XSkpLnRvLmVxdWFsKDMpXG59KVxuIl0sIm5hbWVzIjpbIml0IiwiZXhwZWN0IiwidG8iLCJlcXVhbCIsIk1hdGgiLCJtaW4iXSwic291cmNlUm9vdCI6IiJ9
astone123 marked this conversation as resolved.
Show resolved Hide resolved
`

exports['webpack preprocessor - e2e has less verbose syntax error 1'] = `
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ describe('./lib/cross-origin-callback-loader', () => {
expectAddFileSource(store).to.equal(stripIndent`
__cypressCrossOriginCallback = () => {
const utils = require('../support/utils');

utils.foo();
}`)
})
Expand All @@ -230,9 +229,7 @@ describe('./lib/cross-origin-callback-loader', () => {
`it('test', () => {
cy.origin('http://www.foobar.com:3500', () => {
require('../support/commands')

const utils = require('../support/utils')

const _ = require('lodash')
})
})`,
Expand All @@ -241,9 +238,7 @@ describe('./lib/cross-origin-callback-loader', () => {
expectAddFileSource(store).to.equal(stripIndent`
__cypressCrossOriginCallback = () => {
require('../support/commands');

const utils = require('../support/utils');

const _ = require('lodash');
}`)
})
Expand All @@ -270,9 +265,7 @@ describe('./lib/cross-origin-callback-loader', () => {
`it('test', () => {
cy.origin('http://www.foobar.com:3500', () => {
const someVar = 'someValue'

const result = require('./fn')(someVar)

expect(result).to.equal('mutated someVar')
})
})`,
Expand All @@ -281,9 +274,7 @@ describe('./lib/cross-origin-callback-loader', () => {
expectAddFileSource(store).to.equal(stripIndent`
__cypressCrossOriginCallback = () => {
const someVar = 'someValue';

const result = require('./fn')(someVar);

expect(result).to.equal('mutated someVar');
}`)
})
Expand All @@ -293,7 +284,6 @@ describe('./lib/cross-origin-callback-loader', () => {
`it('test', () => {
cy.origin('http://www.foobar.com:3500', { args: { foo: 'foo'}}, ({ foo }) => {
const result = require('./fn')(foo)

expect(result).to.equal('mutated someVar')
})
})`,
Expand All @@ -304,7 +294,6 @@ describe('./lib/cross-origin-callback-loader', () => {
foo
}) => {
const result = require('./fn')(foo);

expect(result).to.equal('mutated someVar');
}`)
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const { defineConfig: cypressDefineConfig } = require("cypress");

export default cypressDefineConfig({
component: {
devServer: {
Expand Down
1 change: 1 addition & 0 deletions packages/data-context/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"prettier": "2.5.1",
"randexp": "0.5.3",
"randomstring": "1.1.5",
"react-docgen": "6.0.0-alpha.3",
"semver": "7.3.2",
"simple-git": "3.4.0",
"stringify-object": "^3.0.0",
Expand Down
32 changes: 30 additions & 2 deletions packages/data-context/src/actions/ProjectActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,18 @@ import path from 'path'
import assert from 'assert'

import type { ProjectShape } from '../data/coreDataShape'

import type { DataContext } from '..'
import { codeGenerator, SpecOptions, hasNonExampleSpec } from '../codegen'
import templates from '../codegen/templates'
import { insertValuesInConfigFile } from '../util'
import { getError } from '@packages/errors'
import { resetIssuedWarnings } from '@packages/config'
import { WizardFrontendFramework, WIZARD_FRAMEWORKS } from '@packages/scaffold-config'
import { parse as parseReactComponent, resolver as reactDocgenResolvers } from 'react-docgen'
import fs from 'fs-extra'
import Debug from 'debug'

const debug = Debug('cypress:data-context:sources:ProjectActions')

export interface ProjectApiShape {
/**
Expand Down Expand Up @@ -76,6 +80,11 @@ type SetForceReconfigureProjectByTestingType = {
testingType?: TestingType
}

interface ReactComponentDescriptor {
description: string
astone123 marked this conversation as resolved.
Show resolved Hide resolved
displayName: string
}

export class ProjectActions {
constructor (private ctx: DataContext) {}

Expand Down Expand Up @@ -345,7 +354,25 @@ export class ProjectActions {
this.api.insertProjectPreferencesToCache(this.ctx.lifecycleManager.projectTitle, args)
}

async codeGenSpec (codeGenCandidate: string, codeGenType: CodeGenType): Promise<NexusGenUnions['GeneratedSpecResult']> {
// @ts-ignore
async getReactComponentsFromFile (filePath: string): Promise<ReactComponentDescriptor[]> {
try {
const src = await fs.readFile(filePath, 'utf8')
astone123 marked this conversation as resolved.
Show resolved Hide resolved

let result = parseReactComponent(src, reactDocgenResolvers.findAllExportedComponentDefinitions)
astone123 marked this conversation as resolved.
Show resolved Hide resolved
// types appear to be incorrect in [email protected]
// TODO: update when 6.0.0 stable is out for fixed types.
const defs = (Array.isArray(result) ? result : [result]) as ReactComponentDescriptor[]

return defs
} catch (err) {
debug(err)

return []
}
}

async codeGenSpec (codeGenCandidate: string, codeGenType: CodeGenType, componentName?: string): Promise<NexusGenUnions['GeneratedSpecResult']> {
const project = this.ctx.currentProject

assert(project, 'Cannot create spec without currentProject.')
Expand All @@ -371,6 +398,7 @@ export class ProjectActions {
specPattern,
currentProject: this.ctx.currentProject,
specs: this.ctx.project.specs,
componentName,
})

let codeGenOptions = await newSpecCodeGenOptions.getCodeGenOptions()
Expand Down
65 changes: 47 additions & 18 deletions packages/data-context/src/codegen/spec-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { ParsedPath } from 'path'
import type { CodeGenType } from '@packages/graphql/src/gen/nxs.gen'
import type { WizardFrontendFramework } from '@packages/scaffold-config'
import fs from 'fs-extra'
import { upperFirst } from 'lodash'
import { uniq, upperFirst } from 'lodash'
import path from 'path'
import { getDefaultSpecFileName } from '../sources/migration/utils'
import { toPosix } from '../util'
Expand All @@ -16,6 +16,7 @@ interface CodeGenOptions {
currentProject: string | null
framework?: WizardFrontendFramework
specs?: FoundSpec[]
componentName?: string
}

// Spec file extensions that we will preserve when updating the file name
Expand All @@ -28,7 +29,7 @@ interface CodeGenOptions {
export const expectedSpecExtensions = ['.cy', '.spec', '.test', '-spec', '-test', '_spec']

type ComponentExtension = `.cy.${'js' | 'ts' | 'jsx' | 'tsx'}`
type TemplateKey = 'e2e' | 'componentEmpty' | 'vueComponent'
type TemplateKey = 'e2e' | 'componentEmpty' | 'vueComponent' | 'reactComponent'
export class SpecOptions {
private parsedPath: ParsedPath;

Expand All @@ -54,19 +55,14 @@ export class SpecOptions {
throw new Error('Cannot generate a spec without a framework')
}

// This only works for Vue projects right now. If the framework is not Vue, we're generating an empty component test
if (this.options.framework.codeGenFramework !== 'vue') {
return {
codeGenType: this.options.codeGenType,
fileName: await this.buildFileName(),
templateKey: 'componentEmpty' as TemplateKey,
overrideCodeGenDir: '',
}
switch (this.options.framework.codeGenFramework) {
case 'react':
return await this.getReactSpecOptions()
case 'vue':
return await this.getVueSpecOptions()
default:
throw new Error(`Unable to generate spec for ${this.options.framework.codeGenFramework}`)
astone123 marked this conversation as resolved.
Show resolved Hide resolved
}

const frameworkOptions = await this.getFrameworkComponentOptions()

return frameworkOptions
}

private getRelativePathToComponent (specParsedPath?: ParsedPath) {
Expand All @@ -81,7 +77,7 @@ export class SpecOptions {
return `./${this.parsedPath.base}`
}

private async getFrameworkComponentOptions () {
private async getVueSpecOptions () {
const componentName = this.buildComponentNameFromFilename(this.parsedPath.name)

const extension = await this.getVueExtension()
Expand All @@ -92,7 +88,7 @@ export class SpecOptions {
if (!this.options.isDefaultSpecPattern) {
parsedSpecPath = path.parse(await getDefaultSpecFileName({
currentProject: this.options.currentProject,
testingType: this.options.codeGenType === 'componentEmpty' || this.options.codeGenType === 'component' ? 'component' : 'e2e',
testingType: 'component',
fileExtensionToUse: (extension === '.cy.ts' || extension === '.cy.tsx') ? 'ts' : 'js',
specPattern: this.options.specPattern,
name: componentName,
Expand All @@ -112,6 +108,39 @@ export class SpecOptions {
}
}

private async getReactSpecOptions () {
// For React specs, use the component name that the user selected. Otherwise fall back to the component file name.
const componentName = this.options.componentName || this.parsedPath.name

const extension = `.cy${this.parsedPath.ext}`

let parsedSpecPath: ParsedPath | undefined

// If we have a custom spec pattern, write the spec to a path that matches the pattern instead of the component directory
if (!this.options.isDefaultSpecPattern) {
parsedSpecPath = path.parse(await getDefaultSpecFileName({
currentProject: this.options.currentProject,
testingType: 'component',
fileExtensionToUse: (extension === '.cy.ts' || extension === '.cy.tsx') ? 'ts' : 'js',
specPattern: this.options.specPattern,
name: componentName,
specs: this.options.specs }))
}

// The path to import the component from
const componentPath = this.getRelativePathToComponent(parsedSpecPath)

return {
codeGenType: this.options.codeGenType,
componentName,
componentPath,
// If the component name and file name are different, the spec file should be combined (ex: SpecNameComponentName.cy.xx)
fileName: await this.buildComponentSpecFilename(extension, parsedSpecPath, uniq([this.parsedPath.name, componentName]).join('')),
templateKey: 'reactComponent' as TemplateKey,
overrideCodeGenDir: parsedSpecPath?.dir,
}
}

private async getVueExtension (): Promise<ComponentExtension> {
try {
const fileContent = await fs
Expand Down Expand Up @@ -153,11 +182,11 @@ export class SpecOptions {
.join('')
}

private buildComponentSpecFilename (specExt: string, filePath?: ParsedPath) {
private buildComponentSpecFilename (specExt: string, filePath?: ParsedPath, fileName?: string) {
const { dir, base, ext } = filePath || this.parsedPath
const cyWithExt = this.getSpecExtension(filePath) + ext

const name = base.slice(0, -cyWithExt.length)
const name = fileName || base.slice(0, -cyWithExt.length)

const finalExtension = filePath ? cyWithExt : specExt

Expand Down
1 change: 1 addition & 0 deletions packages/data-context/src/codegen/templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import cypressEx from '@packages/example'
const getPath = (dir: string) => path.join(__dirname, dir)

export default {
reactComponent: getPath('./templates/react-component'),
vueComponent: getPath('./templates/vue-component'),
componentEmpty: getPath('./templates/empty-component'),
e2e: getPath('./templates/empty-e2e'),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
fileName: <%= fileName %>
---

import React from 'react'
import { <%- componentName %> } from '<%- componentPath %>'
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens for default export components, like CounterDefault? Would this become import { CounterDefault } from './counter-default' (which would be undefined). Did we find a solution for this, or are we moving forward with this as a known limitation?

It should be possible to write a quick babel parser to find the default export and compare that to the components react-docgen found. Not sure how expensive this would be, probably not that much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we'll need a separate spec template for default export components. I'm looking into how we can figure out if a component is exported by using a custom handler with React Docgen.

Otherwise yeah we can probably do it after we parse with React Docgen

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at the result the componentDefinition which is what the handler receives and it doesn't contain enough info to determine how the component was exported. For example:

const Button = (props: {text: string}) => <button>{props.text}</button>

export { Button }
export default Button

The componentDef only contains info on const Button = (props: {text: string}) => <button>{props.text}</button>. Rather than a custom handler, I believe a custom resolver makes sense here so we can scrape all of the exported declarations and then compare them to the returned componentDefinitions. We don't need to write our own parser since a resolver directly receives the parsed ast.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to handle this as a separate task (to be completed before releasing this feature) #24929


describe('<<%- componentName %> />', () => {
it('renders', () => {
// see: https://on.cypress.io/mounting-react
cy.mount(<<%- componentName %> />)
})
})
1 change: 1 addition & 0 deletions packages/data-context/src/sources/migration/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ export function getSpecPattern (cfg: LegacyCypressConfigJson, testingType: Testi
function formatWithBundledBabel (config: string) {
const ast = parse(config)

// @ts-ignore - transitive babel types have a minor conflict - readonly vs non readonly.
let { code } = generate(ast, {}, config)
// By default babel generates imports like this:
// const {
Expand Down
46 changes: 46 additions & 0 deletions packages/data-context/test/unit/actions/ProjectActions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ProjectActions } from '../../../src/actions/ProjectActions'
import { createTestDataContext } from '../helper'
import { expect } from 'chai'
import sinon from 'sinon'
import path from 'path'
astone123 marked this conversation as resolved.
Show resolved Hide resolved
import { SpecWithRelativeRoot } from '@packages/types'

describe('ProjectActions', () => {
Expand All @@ -17,6 +18,51 @@ describe('ProjectActions', () => {
actions = new ProjectActions(ctx)
})

context('getReactComponentsFromFile', () => {
const absolutePathPrefix = path.resolve('./test/unit/actions/project')

it('returns React components from file with class component', async () => {
const reactComponents = await actions.getReactComponentsFromFile(`${absolutePathPrefix}/counter-class.jsx`)

expect(reactComponents).to.have.length(1)
expect(reactComponents[0].displayName).to.equal('Counter')
})

it('returns React components from file with functional component', async () => {
const reactComponents = await actions.getReactComponentsFromFile(`${absolutePathPrefix}/counter-functional.jsx`)

expect(reactComponents).to.have.length(1)
expect(reactComponents[0].displayName).to.equal('Counter')
})

it('returns only exported React components from file with functional components', async () => {
const reactComponents = await actions.getReactComponentsFromFile(`${absolutePathPrefix}/counter-multiple-components.jsx`)

expect(reactComponents).to.have.length(1)
expect(reactComponents[0].displayName).to.equal('CounterContainer')
})

it('returns React components from a tsx file', async () => {
const reactComponents = await actions.getReactComponentsFromFile(`${absolutePathPrefix}/counter.tsx`)

expect(reactComponents).to.have.length(1)
expect(reactComponents[0].displayName).to.equal('Counter')
})

it('returns React components that are exported by default', async () => {
const reactComponents = await actions.getReactComponentsFromFile(`${absolutePathPrefix}/counter-default.tsx`)

expect(reactComponents).to.have.length(1)
expect(reactComponents[0].displayName).to.equal('CounterDefault')
})

it('does not throw while parsing empty file', async () => {
const reactComponents = await actions.getReactComponentsFromFile(`${absolutePathPrefix}/empty.jsx`)

expect(reactComponents).to.have.length(0)
})
astone123 marked this conversation as resolved.
Show resolved Hide resolved
})

context('hasNonExampleSpec', () => {
context('testing type not set yet', () => {
it('should indicate there are NO non example spec files if empty', async () => {
Expand Down
20 changes: 20 additions & 0 deletions packages/data-context/test/unit/actions/project/counter-class.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react'

export class Counter extends React.Component {
constructor (props) {
super(props)
this.state = {
count: 0,
}
}

click = () => {
this.setState({
count: this.state.count + 1,
})
}

render () {
return <p onClick={this.click}>count: {this.state.count}</p>
}
}
Loading