Skip to content

Commit

Permalink
Merge branch 'main' into fix/30252
Browse files Browse the repository at this point in the history
  • Loading branch information
tienifr committed Nov 13, 2023
2 parents a338986 + 604bdaa commit b33c309
Show file tree
Hide file tree
Showing 103 changed files with 49,214 additions and 28,843 deletions.
12 changes: 0 additions & 12 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,6 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
- [ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
- [ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
- [ ] I verified that if a function's arguments changed that all usages have also been updated correctly
- [ ] If a new component is created I verified that:
- [ ] A similar component doesn't exist in the codebase
- [ ] All props are defined accurately and each prop has a `/** comment above it */`
- [ ] The file is named correctly
- [ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- [ ] The only data being stored in the state is data necessary for rendering and nothing else
- [ ] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
- [ ] For Class Components, any internal methods passed to components event handlers are bound to `this` properly so there are no scoping issues (i.e. for `onClick={this.submit}` the method `this.submit` should be bound to `this` in the constructor)
- [ ] Any internal methods bound to `this` are necessary to be bound (i.e. avoid `this.submit = this.submit.bind(this);` if `this.submit` is never passed to a component event handler like `onClick`)
- [ ] All JSX used for rendering exists in the render method
- [ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
- [ ] If any new file was added I verified that:
- [ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
- [ ] If a new CSS style is added I verified that:
Expand All @@ -116,7 +105,6 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
- [ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
- [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page.
- [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps.
- [ ] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

### Screenshots/Videos
<details>
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/javascript/authorChecklist/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ inputs:
description: Auth token for New Expensify Github
required: true
runs:
using: 'node16'
using: 'node20'
main: './index.js'
67 changes: 0 additions & 67 deletions .github/actions/javascript/authorChecklist/authorChecklist.js

This file was deleted.

158 changes: 158 additions & 0 deletions .github/actions/javascript/authorChecklist/authorChecklist.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import * as core from '@actions/core';
import * as github from '@actions/github';
import escapeRegExp from 'lodash/escapeRegExp';
import CONST from '../../../libs/CONST';
import GithubUtils from '../../../libs/GithubUtils';
import newComponentCategory from './categories/newComponentCategory';

const pathToAuthorChecklist = `https://raw.githubusercontent.com/${CONST.GITHUB_OWNER}/${CONST.APP_REPO}/main/.github/PULL_REQUEST_TEMPLATE.md`;
const checklistStartsWith = '### PR Author Checklist';
const checklistEndsWith = '\r\n### Screenshots/Videos';

const prNumber = github.context.payload.pull_request?.number;

const CHECKLIST_CATEGORIES = {
NEW_COMPONENT: newComponentCategory,
};

/**
* Look at the contents of the pull request, and determine which checklist categories apply.
*/
async function getChecklistCategoriesForPullRequest(): Promise<Set<string>> {
const checks = new Set<string>();
const changedFiles = await GithubUtils.paginate(GithubUtils.octokit.pulls.listFiles, {
owner: CONST.GITHUB_OWNER,
repo: CONST.APP_REPO,
// eslint-disable-next-line @typescript-eslint/naming-convention
pull_number: prNumber,
// eslint-disable-next-line @typescript-eslint/naming-convention
per_page: 100,
});
const possibleCategories = await Promise.all(
Object.values(CHECKLIST_CATEGORIES).map(async (category) => ({
items: category.items,
doesCategoryApply: await category.detect(changedFiles),
})),
);
for (const category of possibleCategories) {
if (category.doesCategoryApply) {
for (const item of category.items) {
checks.add(item);
}
}
}
return checks;
}

function partitionWithChecklist(body: string): string[] {
const [contentBeforeChecklist, contentAfterStartOfChecklist] = body.split(checklistStartsWith);
const [checklistContent, contentAfterChecklist] = contentAfterStartOfChecklist.split(checklistEndsWith);
return [contentBeforeChecklist, checklistContent, contentAfterChecklist];
}

async function getNumberOfItemsFromAuthorChecklist(): Promise<number> {
const response = await fetch(pathToAuthorChecklist);
const fileContents = await response.text();
const checklist = partitionWithChecklist(fileContents)[1];
const numberOfChecklistItems = (checklist.match(/\[ \]/g) ?? []).length;
return numberOfChecklistItems;
}

function checkPRForCompletedChecklist(expectedNumberOfChecklistItems: number, checklist: string) {
const numberOfFinishedChecklistItems = (checklist.match(/- \[x\]/gi) ?? []).length;
const numberOfUnfinishedChecklistItems = (checklist.match(/- \[ \]/g) ?? []).length;

const minCompletedItems = expectedNumberOfChecklistItems - 2;

console.log(`You completed ${numberOfFinishedChecklistItems} out of ${expectedNumberOfChecklistItems} checklist items with ${numberOfUnfinishedChecklistItems} unfinished items`);

if (numberOfFinishedChecklistItems >= minCompletedItems && numberOfUnfinishedChecklistItems === 0) {
console.log('PR Author checklist is complete 🎉');
return;
}

console.log(`Make sure you are using the most up to date checklist found here: ${pathToAuthorChecklist}`);
core.setFailed("PR Author Checklist is not completely filled out. Please check every box to verify you've thought about the item.");
}

async function generateDynamicChecksAndCheckForCompletion() {
// Generate dynamic checks
const dynamicChecks = await getChecklistCategoriesForPullRequest();
let isPassing = true;
let didChecklistChange = false;

const body = github.context.payload.pull_request?.body ?? '';

// eslint-disable-next-line prefer-const
let [contentBeforeChecklist, checklist, contentAfterChecklist] = partitionWithChecklist(body);

for (const check of dynamicChecks) {
// Check if it's already in the PR body, capturing the whether or not it's already checked
const regex = new RegExp(`- \\[([ x])] ${escapeRegExp(check)}`);
const match = regex.exec(checklist);
if (!match) {
// Add it to the PR body
isPassing = false;
checklist += `- [ ] ${check}\r\n`;
didChecklistChange = true;
} else {
const isChecked = match[1] === 'x';
if (!isChecked) {
isPassing = false;
}
}
}

// Check if some dynamic check was added with previous commit, but is not relevant anymore
const allChecks = Object.values(CHECKLIST_CATEGORIES).reduce((acc: string[], category) => acc.concat(category.items), []);

for (const check of allChecks) {
if (!dynamicChecks.has(check)) {
const regex = new RegExp(`- \\[([ x])] ${escapeRegExp(check)}\r\n`);
const match = regex.exec(checklist);
if (match) {
// Remove it from the PR body
checklist = checklist.replace(match[0], '');
didChecklistChange = true;
}
}
}

// Put the PR body back together, need to add the markers back in
const newBody = contentBeforeChecklist + checklistStartsWith + checklist + checklistEndsWith + contentAfterChecklist;

// Update the PR body
if (didChecklistChange) {
await GithubUtils.octokit.pulls.update({
owner: CONST.GITHUB_OWNER,
repo: CONST.APP_REPO,
// eslint-disable-next-line @typescript-eslint/naming-convention
pull_number: prNumber,
body: newBody,
});
console.log('Updated PR checklist');
}

if (!isPassing) {
const err = new Error("New checks were added into checklist. Please check every box to verify you've thought about the item.");
console.error(err);
core.setFailed(err);
}

// check for completion
try {
const numberOfItems = await getNumberOfItemsFromAuthorChecklist();
checkPRForCompletedChecklist(numberOfItems, checklist);
} catch (error) {
console.error(error);
if (error instanceof Error) {
core.setFailed(error.message);
}
}
}

if (require.main === module) {
generateDynamicChecksAndCheckForCompletion();
}

export default generateDynamicChecksAndCheckForCompletion;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type Category = {
detect: (changedFiles: Array<{filename: string; status: string}>) => Promise<boolean>;
items: string[];
};

export default Category;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import Category from './Category';
import newComponent from './newComponentCategory';

const categories: Category[] = [newComponent];

export default categories;
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import github from '@actions/github';
import {parse} from '@babel/parser';
import traverse from '@babel/traverse';
import CONST from '../../../../libs/CONST';
import GithubUtils from '../../../../libs/GithubUtils';
import promiseSome from '../../../../libs/promiseSome';
import Category from './Category';

type SuperClassType = {superClass: {name?: string; object: {name: string}; property: {name: string}} | null; name: string};

const items = [
"I verified that similar component doesn't exist in the codebase",
'I verified that all props are defined accurately and each prop has a `/** comment above it */`',
'I verified that each file is named correctly',
'I verified that each component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone',
'I verified that the only data being stored in component state is data necessary for rendering and nothing else',
"In component if we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes",
'For Class Components, any internal methods passed to components event handlers are bound to `this` properly so there are no scoping issues (i.e. for `onClick={this.submit}` the method `this.submit` should be bound to `this` in the constructor)',
'I verified that component internal methods bound to `this` are necessary to be bound (i.e. avoid `this.submit = this.submit.bind(this);` if `this.submit` is never passed to a component event handler like `onClick`)',
'I verified that all JSX used for rendering exists in the render method',
'I verified that each component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions',
];

function isComponentOrPureComponent(name?: string) {
return name === 'Component' || name === 'PureComponent';
}

function detectReactComponent(code: string, filename: string): boolean | undefined {
if (!code) {
console.error('failed to get code from a filename', code, filename);
return;
}
const ast = parse(code, {
sourceType: 'module',
plugins: ['jsx', 'typescript'], // enable jsx plugin
});

let isReactComponent = false;

traverse(ast, {
enter(path) {
if (isReactComponent) {
return;
}
if (path.isFunctionDeclaration() || path.isArrowFunctionExpression() || path.isFunctionExpression()) {
path.traverse({
// eslint-disable-next-line @typescript-eslint/naming-convention
JSXElement() {
isReactComponent = true;
path.stop();
},
});
}
},
// eslint-disable-next-line @typescript-eslint/naming-convention
ClassDeclaration(path) {
const {superClass} = path.node as unknown as SuperClassType;
if (
superClass &&
((superClass.object && superClass.object.name === 'React' && isComponentOrPureComponent(superClass.property.name)) || isComponentOrPureComponent(superClass.name))
) {
isReactComponent = true;
path.stop();
}
},
});

return isReactComponent;
}

function nodeBase64ToUtf8(data: string) {
return Buffer.from(data, 'base64').toString('utf-8');
}

async function detectReactComponentInFile(filename: string): Promise<boolean | undefined> {
const params = {
owner: CONST.GITHUB_OWNER,
repo: CONST.APP_REPO,
path: filename,
ref: github.context.payload.pull_request?.head.ref,
};
try {
const {data} = await GithubUtils.octokit.repos.getContent(params);
const content = 'content' in data ? nodeBase64ToUtf8(data.content || '') : data;
return detectReactComponent(content, filename);
} catch (error) {
console.error('An unknown error occurred with the GitHub API: ', error, params);
}
}

async function detect(changedFiles: Array<{filename: string; status: string}>): Promise<boolean> {
const filteredFiles = changedFiles.filter(({filename, status}) => status === 'added' && (filename.endsWith('.js') || filename.endsWith('.ts') || filename.endsWith('.tsx')));
try {
await promiseSome(
filteredFiles.map(({filename}) => detectReactComponentInFile(filename)),
(result) => !!result,
);
return true;
} catch (err) {
return false;
}
}

const newComponentCategory: Category = {
detect,
items,
};

export default newComponentCategory;
export {detectReactComponent};
Loading

0 comments on commit b33c309

Please sign in to comment.