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: block compilation on codemod comments and ask to remove #71103

Merged
merged 14 commits into from
Oct 11, 2024
Prev Previous commit
Next Next commit
give hint of codemod ignore in swc
  • Loading branch information
huozhi committed Oct 11, 2024
commit 236a900c7b51c581e914222318427138333d18b1
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ where
}

// declare a const of comment prefix
const COMMENT_PREFIX: &str = "@next-codemod-error";
const COMMENT_ERROR_PREFIX: &str = "@next-codemod-error";
const COMMENT_BYPASS_PREFIX: &str = "@next-codemod-ignore";

impl<C> LintErrorComment<C>
where
Expand All @@ -31,7 +32,7 @@ where
let trimmed_text = comment.text.trim();
// if comment contains @next/codemod comment "@next-codemod-error",
// report an error from the linter to fail the build
if trimmed_text.contains(COMMENT_PREFIX) {
if trimmed_text.contains(COMMENT_ERROR_PREFIX) {
let span = if is_leading {
comment
.span
Expand All @@ -41,11 +42,14 @@ where
.span
.with_hi(comment.span.hi() + swc_core::common::BytePos(1))
};
let action = trimmed_text.replace(COMMENT_PREFIX, "");
let action = trimmed_text.replace(COMMENT_ERROR_PREFIX, "");
let err_message = format!(
"You have unresolved @next/codemod comments that need to be reviewed, please \
address and remove them to proceed with the build.\nAction: \"{}\"",
action
"You have unresolved @next/codemod comment \"{}\" that need to be reviewed, \
please address and remove them to proceed with the build.\nYou can also bypass \
the build error by replacing \"{}\" with \"{}\".",
action.trim(),
COMMENT_ERROR_PREFIX,
COMMENT_BYPASS_PREFIX
);
report(span, &err_message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,52 +29,76 @@ describe('app-dir - error-on-next-codemod-comment', () => {
Ecmascript file had an error
1 | export default function Page() {
> 2 | // @next-codemod-error remove jsx of next line
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

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

aside: column is off by one here. We also have other tests where we observe this. Just making a mental note for when we revisit the error overlay.

3 | return <p>hello world</p>
4 | }
5 |

You have unresolved @next/codemod comments that need to be reviewed, please address and remove them to proceed with the build.
Action: " remove jsx of next line""
You have unresolved @next/codemod comment "remove jsx of next line" that need to be reviewed, please address and remove them to proceed with the build.
You can also bypass the build error by replacing "@next-codemod-error" with "@next-codemod-ignore"."
`)
} else {
expect(await getRedboxSource(browser)).toMatchInlineSnapshot(`
"./app/page.tsx
Error: x You have unresolved @next/codemod comment needs to be removed, please address and remove it to proceed build.
| Action: " remove jsx of next line"
Error: x You have unresolved @next/codemod comment "remove jsx of next line" that need to be reviewed, please address and remove them to proceed with the build.
| You can also bypass the build error by replacing "@next-codemod-error" with "@next-codemod-ignore".
,-[2:1]
1 | export default function Page() {
2 | // @next-codemod-error remove jsx of next line
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3 | return <p>hello world</p>
4 | }
\`----"
`)
}
})

it('should disappear the error when you remove the codemod comment', async () => {
it('should disappear the error when you rre the codemod comment', async () => {
const browser = await next.browser('/')

await assertHasRedbox(browser)

await next.patchFile(
'app/page.tsx',
`
export default function Page() { return <p>hello world</p> }
`
)
let originFileContent
await next.patchFile('app/page.tsx', (code) => {
originFileContent = code
return code.replace(
'// @next-codemod-error remove jsx of next line',
''
)
})

await retry(async () => {
await assertNoRedbox(browser)
})

// Recover the original file content
await next.patchFile('app/page.tsx', originFileContent)
})

it('should disappear the error when you replace with bypass comment', async () => {
const browser = await next.browser('/')

await assertHasRedbox(browser)

let originFileContent
await next.patchFile('app/page.tsx', (code) => {
originFileContent = code
return code.replace('@next-codemod-error', '@next-codemod-bypass')
})

await retry(async () => {
await assertNoRedbox(browser)
})

// Recover the original file content
await next.patchFile('app/page.tsx', originFileContent)
})
} else {
it('should fail the build with next build', async () => {
const res = await next.build()
expect(res.exitCode).toBe(1)
expect(res.cliOutput).toContain(
'You have unresolved @next/codemod comments that need to be reviewed, please address and remove them to proceed with the build.'
'You have unresolved @next/codemod comment "remove jsx of next line" that need to be reviewed, please address and remove them to proceed with the build'
)
})
}
Expand Down
6 changes: 0 additions & 6 deletions test/e2e/app-dir/error-on-next-codemod-comment/next.config.js

This file was deleted.