Skip to content

Commit

Permalink
fix(WEB-7151): Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
georgegillams committed Nov 14, 2023
1 parent c812a54 commit 34ff70c
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 50 deletions.
23 changes: 7 additions & 16 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

31 changes: 17 additions & 14 deletions src/checks/requiredTypeScript.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ describe('Required TypeScript check', () => {
pullRequestResponse as PullsGetResponse
)

mockGithub.downloadContent.mockResolvedValue({
content: 'some js/ts content\nsome more js/ts content',
} as Content)
mockReadFile.mockReturnValue(
'some js/ts content\nsome more js/ts content'
)
})

it('returns true for PR without JS changes and good tsconfig', async () => {
Expand Down Expand Up @@ -90,19 +90,22 @@ describe('Required TypeScript check', () => {
mockGithub.getPullRequestFiles.mockResolvedValue([
{ filename: 'filename.js', additions: 100, deletions: 20 },
] as PullRequestFiles)
mockGithub.downloadContent.mockResolvedValue({
content: '// @ts-check\nsome js/ts content\nsome more js/ts content',
} as Content)

mockGlob.mockResolvedValue(['tsconfig.json'])
mockReadFile.mockReturnValue(
JSON.stringify({
compilerOptions: {
allowUnreachableCode: false,
noImplicitAny: true,
},
})
)
mockReadFile.mockImplementation((filename: string) => {
switch (filename) {
case 'tsconfig.json':
return JSON.stringify({
compilerOptions: {
allowUnreachableCode: false,
noImplicitAny: true,
},
})

default:
return '// @ts-check\nsome js/ts content\nsome more js/ts content'
}
})

await expect(requiredTypeScript.run()).resolves.toBeTruthy()
})
Expand Down
27 changes: 8 additions & 19 deletions src/checks/requiredTypeScript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { isBot } from '../conditions/triggeredByBot'

import Check from './check'

const JS_TS_CHECK_COMMENT = '// @ts-check'
const JS_TS_CHECK_COMMENT_REGEX = /^\s*\/\/\s*@ts-check/gm

type TsConfig = {
compilerOptions?: {
Expand Down Expand Up @@ -40,16 +40,9 @@ const requiredTypeScript: Check = {
}
export default requiredTypeScript

const getFileContent = async (filename: string) => {
const getFileContent = (filename: string) => {
try {
const response = await github.downloadContent(filename, github.context.ref)
if (!('content' in response)) {
throw new Error('No content in response')
}

return response.encoding === 'base64'
? Buffer.from(response.content, 'base64').toString()
: response.content
return fs.readFile(filename)
} catch (_) {
return ''
}
Expand All @@ -76,12 +69,7 @@ async function checkPullRequest(): Promise<boolean> {

const files = await github.getPullRequestFiles(pr.number)
const filesWithContent = await Promise.all(
files.map((f) =>
(async () => {
const fileContent = await getFileContent(f.filename)
return { ...f, fileContent }
})()
)
files.map((f) => ({ ...f, fileContent: getFileContent(f.filename) }))
)
const forbiddenJsFiles = filesWithContent.filter((f, index) =>
isForbiddenJSFile(f.filename, f.fileContent, filter)
Expand Down Expand Up @@ -180,7 +168,7 @@ export async function measureTsAdoption(
})
const typedJsFiles = jsFiles.filter((file) => {
const fileContent = fs.readFile(file)
return fileContent.startsWith(JS_TS_CHECK_COMMENT)
return !!fileContent.match(JS_TS_CHECK_COMMENT_REGEX)
})
const tsFiles = await fs.glob({
patterns: ['**/*.ts', '**/*.tsx'],
Expand Down Expand Up @@ -240,8 +228,9 @@ export function isForbiddenJSFile(
): boolean {
const jsPattern = /\.jsx?$/i
const hasJsExtension = jsPattern.test(filename) && !filter.ignores(filename)
const appliesTypescriptViaComment =
fileContent.startsWith(JS_TS_CHECK_COMMENT)
const appliesTypescriptViaComment = !!fileContent.match(
JS_TS_CHECK_COMMENT_REGEX
)

return hasJsExtension && !appliesTypescriptViaComment
}
Expand Down

0 comments on commit 34ff70c

Please sign in to comment.