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.

Sign up for GitHub

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

Jump to bottom

sweep: Turn all class components into functional components #6

Open
kevinlu1248 opened this issue Jul 20, 2023 · 1 comment · May be fixed by #7
Open

sweep: Turn all class components into functional components #6

kevinlu1248 opened this issue Jul 20, 2023 · 1 comment · May be fixed by #7
Labels
sweep Assigns Sweep to an issue or pull request.

Comments

@kevinlu1248
Copy link
Member

No description provided.

@ghost ghost added the sweep Assigns Sweep to an issue or pull request. label Jul 20, 2023
@ghost
Copy link

ghost commented Jul 20, 2023 •
edited by ghost
Loading

Here's the PR! #7.

💎 Sweep Pro: I used GPT-4 to create this ticket. You have 59 GPT-4 tickets left.

Step 1: 🔍 Code Search

I found the following snippets in your repository. I will now analyze these snippets and come up with a plan.

Some code snippets I looked at (click to expand). If some file is missing from here, you can mention the path in the ticket description.

create-react-app/packages/react-scripts/fixtures/kitchensink/template/src/App.js

Lines 1 to 114 in 0a827f6

/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import React, { Component, createElement } from 'react';
import PropTypes from 'prop-types';
class BuiltEmitter extends Component {
static propTypes = {
error: PropTypes.string,
feature: PropTypes.func,
};
componentDidMount() {
const { error, feature } = this.props;
if (error) {
this.handleError(error);
return;
}
// Class components must call this.props.onReady when they're ready for the test.
// We will assume functional components are ready immediately after mounting.
if (!Object.prototype.isPrototypeOf.call(Component, feature)) {
this.handleReady();
}
}
handleError(error) {
document.dispatchEvent(new Event('ReactFeatureError'));
}
handleReady() {
document.dispatchEvent(new Event('ReactFeatureDidMount'));
}
render() {
const {
props: { feature },
handleReady,
} = this;
return (
<div>
{feature &&
createElement(feature, {
onReady: handleReady,
})}
</div>
);
}
}
class App extends Component {
constructor(props) {
super(props);
this.state = { feature: null };
this.setFeature = this.setFeature.bind(this);
}
componentDidMount() {
const url = window.location.href;
// const feature = window.location.hash.slice(1);
// This works around an issue of a duplicate hash in the href
// Ex: http://localhost:3001/#array-destructuring#array-destructuring
// This seems like a jsdom bug as the URL in initDom.js appears to be correct
const feature = url.slice(url.lastIndexOf('#') + 1);
switch (feature) {
case 'array-destructuring':
import('./features/syntax/ArrayDestructuring').then(f =>
this.setFeature(f.default)
);
break;
case 'array-spread':
import('./features/syntax/ArraySpread').then(f =>
this.setFeature(f.default)
);
break;
case 'async-await':
import('./features/syntax/AsyncAwait').then(f =>
this.setFeature(f.default)
);
break;
case 'class-properties':
import('./features/syntax/ClassProperties').then(f =>
this.setFeature(f.default)
);
break;
case 'computed-properties':
import('./features/syntax/ComputedProperties').then(f =>
this.setFeature(f.default)
);
break;
case 'css-inclusion':
import('./features/webpack/CssInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'css-modules-inclusion':
import('./features/webpack/CssModulesInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'scss-inclusion':
import('./features/webpack/ScssInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'scss-modules-inclusion':

create-react-app/packages/create-react-app/createReactApp.js

Lines 358 to 463 in 0a827f6

console.log(chalk.yellow('Falling back to the local Yarn cache.'));
console.log();
}
} else {
command = 'npm';
args = [
'install',
'--no-audit', // https://github.com/facebook/create-react-app/issues/11174
'--save',
'--save-exact',
'--loglevel',
'error',
].concat(dependencies);
if (usePnp) {
console.log(chalk.yellow("NPM doesn't support PnP."));
console.log(chalk.yellow('Falling back to the regular installs.'));
console.log();
}
}
if (verbose) {
args.push('--verbose');
}
const child = spawn(command, args, { stdio: 'inherit' });
child.on('close', code => {
if (code !== 0) {
reject({
command: `${command} ${args.join(' ')}`,
});
return;
}
resolve();
});
});
}
function run(
root,
appName,
version,
verbose,
originalDirectory,
template,
useYarn,
usePnp
) {
Promise.all([
getInstallPackage(version, originalDirectory),
getTemplateInstallPackage(template, originalDirectory),
]).then(([packageToInstall, templateToInstall]) => {
const allDependencies = ['react', 'react-dom', packageToInstall];
console.log('Installing packages. This might take a couple of minutes.');
Promise.all([
getPackageInfo(packageToInstall),
getPackageInfo(templateToInstall),
])
.then(([packageInfo, templateInfo]) =>
checkIfOnline(useYarn).then(isOnline => ({
isOnline,
packageInfo,
templateInfo,
}))
)
.then(({ isOnline, packageInfo, templateInfo }) => {
let packageVersion = semver.coerce(packageInfo.version);
const templatesVersionMinimum = '3.3.0';
// Assume compatibility if we can't test the version.
if (!semver.valid(packageVersion)) {
packageVersion = templatesVersionMinimum;
}
// Only support templates when used alongside new react-scripts versions.
const supportsTemplates = semver.gte(
packageVersion,
templatesVersionMinimum
);
if (supportsTemplates) {
allDependencies.push(templateToInstall);
} else if (template) {
console.log('');
console.log(
`The ${chalk.cyan(packageInfo.name)} version you're using ${
packageInfo.name === 'react-scripts' ? 'is not' : 'may not be'
} compatible with the ${chalk.cyan('--template')} option.`
);
console.log('');
}
console.log(
`Installing ${chalk.cyan('react')}, ${chalk.cyan(
'react-dom'
)}, and ${chalk.cyan(packageInfo.name)}${
supportsTemplates ? ` with ${chalk.cyan(templateInfo.name)}` : ''
}...`
);
console.log();
return install(
root,
useYarn,

create-react-app/docusaurus/docs/running-tests.md

Lines 16 to 174 in 0a827f6

We recommend that you use a separate tool for browser end-to-end tests if you need them. They are beyond the scope of Create React App.
## Filename Conventions
Jest will look for test files with any of the following popular naming conventions:
- Files with `.js` suffix in `__tests__` folders.
- Files with `.test.js` suffix.
- Files with `.spec.js` suffix.
The `.test.js` / `.spec.js` files (or the `__tests__` folders) can be located at any depth under the `src` top level folder.
We recommend to put the test files (or `__tests__` folders) next to the code they are testing so that relative imports appear shorter. For example, if `App.test.js` and `App.js` are in the same folder, the test only needs to `import App from './App'` instead of a long relative path. Collocation also helps find tests more quickly in larger projects.
## Command Line Interface
When you run `npm test`, Jest will launch in watch mode<sup>\*</sup>. Every time you save a file, it will re-run the tests, like how `npm start` recompiles the code.
The watcher includes an interactive command-line interface with the ability to run all tests, or focus on a search pattern. It is designed this way so that you can keep it open and enjoy fast re-runs. You can learn the commands from the “Watch Usage” note that the watcher prints after every run:
![Jest watch mode](https://jestjs.io/img/blog/15-watch.gif)
> \*Although we recommend running your tests in watch mode during development, you can disable this behavior by passing in the `--watchAll=false` flag. In most CI environments, this is handled for you (see [On CI servers](#on-ci-servers)).
## Version Control Integration
By default, when you run `npm test`, Jest will only run the tests related to files changed since the last commit. This is an optimization designed to make your tests run fast regardless of how many tests you have. However it assumes that you don’t often commit the code that doesn’t pass the tests.
Jest will always explicitly mention that it only ran tests related to the files changed since the last commit. You can also press `a` in the watch mode to force Jest to run all tests.
Jest will always run all tests on a [continuous integration](#continuous-integration) server or if the project is not inside a Git or Mercurial repository.
## Writing Tests
To create tests, add `it()` (or `test()`) blocks with the name of the test and its code. You may optionally wrap them in `describe()` blocks for logical grouping but this is neither required nor recommended.
Jest provides a built-in `expect()` global function for making assertions. A basic test could look like this:
```js
import sum from './sum';
it('sums numbers', () => {
expect(sum(1, 2)).toEqual(3);
expect(sum(2, 2)).toEqual(4);
});
```
All `expect()` matchers supported by Jest are [extensively documented here](https://jestjs.io/docs/expect).
You can also use [`jest.fn()` and `expect(fn).toBeCalled()`](https://jestjs.io/docs/expect#tohavebeencalled) to create “spies” or mock functions.
## Testing Components
There is a broad spectrum of component testing techniques. They range from a “smoke test” verifying that a component renders without throwing, to shallow rendering and testing some of the output, to full rendering and testing component lifecycle and state changes.
Different projects choose different testing tradeoffs based on how often components change, and how much logic they contain. If you haven’t decided on a testing strategy yet, we recommend that you start with creating basic smoke tests for your components:
```js
import React from 'react';
import ReactDOM from 'react-dom';
import App from './App';
it('renders without crashing', () => {
const div = document.createElement('div');
ReactDOM.render(<App />, div);
});
```
This test mounts a component and makes sure that it didn’t throw during rendering. Tests like this provide a lot of value with very little effort so they are great as a starting point, and this is the test you will find in `src/App.test.js`.
When you encounter bugs caused by changing components, you will gain a deeper insight into which parts of them are worth testing in your application. This might be a good time to introduce more specific tests asserting specific expected output or behavior.
### React Testing Library
If you’d like to test components in isolation from the child components they render, we recommend using `react-testing-library`. [`react-testing-library`](https://github.com/testing-library/react-testing-library) is a library for testing React components in a way that resembles the way the components are used by end users. It is well suited for unit, integration, and end-to-end testing of React components and applications. It works more directly with DOM nodes, and therefore it's recommended to use with [`jest-dom`](https://github.com/testing-library/jest-dom) for improved assertions.
To install `react-testing-library` and `jest-dom`, you can run:
```sh
npm install --save @testing-library/react @testing-library/jest-dom
```
Alternatively you may use `yarn`:
```sh
yarn add @testing-library/react @testing-library/jest-dom
```
If you want to avoid boilerplate in your test files, you can create a [`src/setupTests.js`](#initializing-test-environment) file:
```js
// react-testing-library renders your components to document.body,
// this adds jest-dom's custom assertions
import '@testing-library/jest-dom';
```
Here's an example of using `react-testing-library` and `jest-dom` for testing that the `<App />` component renders "Learn React".
```js
import React from 'react';
import { render, screen } from '@testing-library/react';
import App from './App';
it('renders welcome message', () => {
render(<App />);
expect(screen.getByText('Learn React')).toBeInTheDocument();
});
```
Learn more about the utilities provided by `react-testing-library` to facilitate testing asynchronous interactions as well as selecting form elements from the [`react-testing-library` documentation](https://testing-library.com/react) and [examples](https://codesandbox.io/s/github/kentcdodds/react-testing-library-examples).
## Using Third Party Assertion Libraries
We recommend that you use `expect()` for assertions and `jest.fn()` for spies. If you are having issues with them please [file those against Jest](https://github.com/facebook/jest/issues/new), and we’ll fix them. We intend to keep making them better for React, supporting, for example, [pretty-printing React elements as JSX](https://github.com/facebook/jest/pull/1566).
However, if you are used to other libraries, such as [Chai](https://www.chaijs.com/) and [Sinon](https://sinonjs.org/), or if you have existing code using them that you’d like to port over, you can import them normally like this:
```js
import sinon from 'sinon';
import { expect } from 'chai';
```
and then use them in your tests like you normally do.
## Initializing Test Environment
> Note: this feature is available with `[email protected]` and higher.
If your app uses a browser API that you need to mock in your tests or if you need a global setup before running your tests, add a `src/setupTests.js` to your project. It will be automatically executed before running your tests.
For example:
### `src/setupTests.js`
```js
const localStorageMock = {
getItem: jest.fn(),
setItem: jest.fn(),
removeItem: jest.fn(),
clear: jest.fn(),
};
global.localStorage = localStorageMock;
```
> Note: Keep in mind that if you decide to "eject" before creating `src/setupTests.js`, the resulting `package.json` file won't contain any reference to it, so you should manually create the property `setupFilesAfterEnv` in the configuration for Jest, something like the following:
> ```js
> "jest": {
> // ...
> "setupFilesAfterEnv": ["<rootDir>/src/setupTests.js"]
> }
> ```
## Focusing and Excluding Tests
You can replace `it()` with `xit()` to temporarily exclude a test from being executed.
Similarly, `fit()` lets you focus on a specific test without running any other tests.

create-react-app/packages/react-scripts/scripts/init.js

Lines 250 to 416 in 0a827f6

'utf8'
);
} catch (err) {
// Silencing the error. As it fall backs to using default npm commands.
}
}
const gitignoreExists = fs.existsSync(path.join(appPath, '.gitignore'));
if (gitignoreExists) {
// Append if there's already a `.gitignore` file there
const data = fs.readFileSync(path.join(appPath, 'gitignore'));
fs.appendFileSync(path.join(appPath, '.gitignore'), data);
fs.unlinkSync(path.join(appPath, 'gitignore'));
} else {
// Rename gitignore after the fact to prevent npm from renaming it to .npmignore
// See: https://github.com/npm/npm/issues/1862
fs.moveSync(
path.join(appPath, 'gitignore'),
path.join(appPath, '.gitignore'),
[]
);
}
// Initialize git repo
let initializedGit = false;
if (tryGitInit()) {
initializedGit = true;
console.log();
console.log('Initialized a git repository.');
}
let command;
let remove;
let args;
if (useYarn) {
command = 'yarnpkg';
remove = 'remove';
args = ['add'];
} else {
command = 'npm';
remove = 'uninstall';
args = [
'install',
'--no-audit', // https://github.com/facebook/create-react-app/issues/11174
'--save',
verbose && '--verbose',
].filter(e => e);
}
// Install additional template dependencies, if present.
const dependenciesToInstall = Object.entries({
...templatePackage.dependencies,
...templatePackage.devDependencies,
});
if (dependenciesToInstall.length) {
args = args.concat(
dependenciesToInstall.map(([dependency, version]) => {
return `${dependency}@${version}`;
})
);
}
// Install react and react-dom for backward compatibility with old CRA cli
// which doesn't install react and react-dom along with react-scripts
if (!isReactInstalled(appPackage)) {
args = args.concat(['react', 'react-dom']);
}
// Install template dependencies, and react and react-dom if missing.
if ((!isReactInstalled(appPackage) || templateName) && args.length > 1) {
console.log();
console.log(`Installing template dependencies using ${command}...`);
const proc = spawn.sync(command, args, { stdio: 'inherit' });
if (proc.status !== 0) {
console.error(`\`${command} ${args.join(' ')}\` failed`);
return;
}
}
if (args.find(arg => arg.includes('typescript'))) {
console.log();
verifyTypeScriptSetup();
}
// Remove template
console.log(`Removing template package using ${command}...`);
console.log();
const proc = spawn.sync(command, [remove, templateName], {
stdio: 'inherit',
});
if (proc.status !== 0) {
console.error(`\`${command} ${args.join(' ')}\` failed`);
return;
}
// Create git commit if git repo was initialized
if (initializedGit && tryGitCommit(appPath)) {
console.log();
console.log('Created git commit.');
}
// Display the most elegant way to cd.
// This needs to handle an undefined originalDirectory for
// backward compatibility with old global-cli's.
let cdpath;
if (originalDirectory && path.join(originalDirectory, appName) === appPath) {
cdpath = appName;
} else {
cdpath = appPath;
}
// Change displayed command to yarn instead of yarnpkg
const displayedCommand = useYarn ? 'yarn' : 'npm';
console.log();
console.log(`Success! Created ${appName} at ${appPath}`);
console.log('Inside that directory, you can run several commands:');
console.log();
console.log(chalk.cyan(` ${displayedCommand} start`));
console.log(' Starts the development server.');
console.log();
console.log(
chalk.cyan(` ${displayedCommand} ${useYarn ? '' : 'run '}build`)
);
console.log(' Bundles the app into static files for production.');
console.log();
console.log(chalk.cyan(` ${displayedCommand} test`));
console.log(' Starts the test runner.');
console.log();
console.log(
chalk.cyan(` ${displayedCommand} ${useYarn ? '' : 'run '}eject`)
);
console.log(
' Removes this tool and copies build dependencies, configuration files'
);
console.log(
' and scripts into the app directory. If you do this, you can’t go back!'
);
console.log();
console.log('We suggest that you begin by typing:');
console.log();
console.log(chalk.cyan(' cd'), cdpath);
console.log(` ${chalk.cyan(`${displayedCommand} start`)}`);
if (readmeExists) {
console.log();
console.log(
chalk.yellow(
'You had a `README.md` file, we renamed it to `README.old.md`'
)
);
}
console.log();
console.log('Happy hacking!');
};
function isReactInstalled(appPackage) {
const dependencies = appPackage.dependencies || {};
return (
typeof dependencies.react !== 'undefined' &&
typeof dependencies['react-dom'] !== 'undefined'
);
}

create-react-app/packages/react-error-overlay/src/containers/StackTrace.js

Lines 1 to 82 in 0a827f6

/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
/* @flow */
import React, { Component } from 'react';
import StackFrame from './StackFrame';
import Collapsible from '../components/Collapsible';
import { isInternalFile } from '../utils/isInternalFile';
import { isBultinErrorName } from '../utils/isBultinErrorName';
import type { StackFrame as StackFrameType } from '../utils/stack-frame';
import type { ErrorLocation } from '../utils/parseCompileError';
const traceStyle = {
fontSize: '1em',
flex: '0 1 auto',
minHeight: '0px',
overflow: 'auto',
};
type Props = {|
stackFrames: StackFrameType[],
errorName: string,
contextSize: number,
editorHandler: (errorLoc: ErrorLocation) => void,
|};
class StackTrace extends Component<Props> {
renderFrames() {
const { stackFrames, errorName, contextSize, editorHandler } = this.props;
const renderedFrames = [];
let hasReachedAppCode = false,
currentBundle = [],
bundleCount = 0;
stackFrames.forEach((frame, index) => {
const { fileName, _originalFileName: sourceFileName } = frame;
const isInternalUrl = isInternalFile(sourceFileName, fileName);
const isThrownIntentionally = !isBultinErrorName(errorName);
const shouldCollapse =
isInternalUrl && (isThrownIntentionally || hasReachedAppCode);
if (!isInternalUrl) {
hasReachedAppCode = true;
}
const frameEle = (
<StackFrame
key={'frame-' + index}
frame={frame}
contextSize={contextSize}
critical={index === 0}
showCode={!shouldCollapse}
editorHandler={editorHandler}
/>
);
const lastElement = index === stackFrames.length - 1;
if (shouldCollapse) {
currentBundle.push(frameEle);
}
if (!shouldCollapse || lastElement) {
if (currentBundle.length === 1) {
renderedFrames.push(currentBundle[0]);
} else if (currentBundle.length > 1) {
bundleCount++;
renderedFrames.push(
<Collapsible key={'bundle-' + bundleCount}>
{currentBundle}
</Collapsible>
);
}
currentBundle = [];
}
if (!shouldCollapse) {
renderedFrames.push(frameEle);

Step 2: 🧐 Snippet Analysis

From looking through the relevant snippets, I decided to make the following modifications:

File Path Proposed Changes
packages/react-scripts/fixtures/kitchensink/template/src/App.js Convert the App and BuiltEmitter class components into functional components. Replace lifecycle methods with appropriate hooks (useEffect for componentDidMount, etc.). Replace this.state and this.setState with useState hook.
packages/react-error-overlay/src/containers/StackTrace.js Convert the StackTrace class component into a functional component. Replace lifecycle methods with appropriate hooks (useEffect for componentDidMount, etc.). Replace this.state and this.setState with useState hook.

Step 3: 📝 Planning

I have created a plan for writing the pull request. I am now working my plan and coding the required changes to address this issue. Here is the planned pull request:

Convert class components to functional components
sweep/convert-class-to-functional

Description

This PR converts the class components in the codebase to functional components. This improves the codebase by using the latest React features and simplifying the component structure.

Changes Made

  • Converted the App and BuiltEmitter components in packages/react-scripts/fixtures/kitchensink/template/src/App.js into functional components.
  • Converted the StackTrace component in packages/react-error-overlay/src/containers/StackTrace.js into a functional component.

Checklist

  • All tests pass successfully.
  • The code follows the project's coding conventions.
  • The code is properly formatted and documented.
  • The changes have been tested locally.

Screenshots (if applicable)

N/A

Related Issues

Closes #6

Step 4: ⌨️ Coding

I have finished coding the issue. I am now reviewing it for completeness.

Step 5: 🔁 Code Review

Success! 🚀

I'm a bot that handles simple bugs and feature requests but I might make mistakes. Please be kind!
Join Our Discord

@ghost ghost linked a pull request Jul 20, 2023 that will close this issue
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
sweep Assigns Sweep to an issue or pull request.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Convert class components to functional components sweepai/create-react-app
1 participant
@kevinlu1248