Skip to content

Commit

Permalink
Improve error messages (#41669)
Browse files Browse the repository at this point in the history
Same as #41636 but with a new condition.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: Jiachi Liu <inbox@huozhi.im>
  • Loading branch information
shuding and huozhi committed Oct 23, 2022
1 parent a1a26ca commit 0a97d07
Show file tree
Hide file tree
Showing 11 changed files with 16 additions and 168 deletions.
9 changes: 2 additions & 7 deletions packages/next-swc/crates/core/src/react_server_components.rs
Expand Up @@ -395,14 +395,9 @@ pub fn server_components<C: Comments>(
JsWord::from("client-only"),
JsWord::from("react-dom/client"),
JsWord::from("react-dom/server"),
// TODO-APP: JsWord::from("next/router"),
// TODO-APP: Rule out client hooks.
],
invalid_client_imports: vec![
JsWord::from("server-only"),
// TODO-APP: Rule out server hooks such as `useCookies`, `useHeaders`,
// `usePreviewData`.
JsWord::from("next/router"),
],
invalid_client_imports: vec![JsWord::from("server-only"), JsWord::from("next/headers")],
invalid_server_react_dom_apis: vec![
JsWord::from("findDOMNode"),
JsWord::from("flushSync"),
Expand Down
Expand Up @@ -25,14 +25,20 @@ export function formatRSCErrorMessage(
'\n\nMaybe one of these should be marked as a client entry with "use client":\n'
} else if (NEXT_RSC_ERR_SERVER_IMPORT.test(message)) {
const matches = message.match(NEXT_RSC_ERR_SERVER_IMPORT)
if (matches && matches[1] === 'react-dom/server') {
// If importing "react-dom/server", we should show a different error.
formattedMessage = `\n\nYou're importing a component that imports react-dom/server. To fix it, render or return the content directly as a Server Component instead for perf and security.`
} else {
formattedMessage = message.replace(
NEXT_RSC_ERR_SERVER_IMPORT,
`\n\nYou're importing a component that imports $1. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.\n\n`
)
switch (matches && matches[1]) {
case 'react-dom/server':
// If importing "react-dom/server", we should show a different error.
formattedMessage = `\n\nYou're importing a component that imports react-dom/server. To fix it, render or return the content directly as a Server Component instead for perf and security.`
break
case 'next/router':
// If importing "next/router", we should tell them to use "next/navigation".
formattedMessage = `\n\nYou have a Server Component that imports next/router. Use next/navigation instead.`
break
default:
formattedMessage = message.replace(
NEXT_RSC_ERR_SERVER_IMPORT,
`\n\nYou're importing a component that imports $1. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.\n\n`
)
}
formattedVerboseMessage =
'\n\nMaybe one of these should be marked as a client entry "use client":\n'
Expand Down
10 changes: 0 additions & 10 deletions test/e2e/app-dir/app/app/hooks/use-cookies/client/page.js

This file was deleted.

10 changes: 0 additions & 10 deletions test/e2e/app-dir/app/app/hooks/use-headers/client/page.js

This file was deleted.

10 changes: 0 additions & 10 deletions test/e2e/app-dir/app/app/hooks/use-preview-data/client/page.js

This file was deleted.

17 changes: 0 additions & 17 deletions test/e2e/app-dir/app/app/old-router/client-router.js

This file was deleted.

7 changes: 0 additions & 7 deletions test/e2e/app-dir/app/app/old-router/is-null.js

This file was deleted.

12 changes: 0 additions & 12 deletions test/e2e/app-dir/app/app/old-router/page.js

This file was deleted.

19 changes: 0 additions & 19 deletions test/e2e/app-dir/app/app/old-router/router.js

This file was deleted.

15 changes: 0 additions & 15 deletions test/e2e/app-dir/app/app/old-router/server-router.js

This file was deleted.

53 changes: 0 additions & 53 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -893,23 +893,6 @@ describe('app dir', () => {
})

describe('next/router', () => {
// `useRouter` should not be accessible in server components.
it('should always return null when accessed from /app', async () => {
const browser = await webdriver(next.url, '/old-router')

try {
await browser.waitForElementByCss('#old-router')

const notNull = await browser.elementsByCss('.was-not-null')
expect(notNull.length).toBe(0)

const wasNull = await browser.elementsByCss('.was-null')
expect(wasNull.length).toBe(6)
} finally {
await browser.close()
}
})

it('should support router.back and router.forward', async () => {
const browser = await webdriver(next.url, '/back-forward/1')

Expand Down Expand Up @@ -1134,42 +1117,6 @@ describe('app dir', () => {

describe('client components', () => {
describe('hooks', () => {
describe('cookies function', () => {
// TODO-APP: should enable when implemented
it.skip('should throw an error when imported', async () => {
const res = await fetchViaHTTP(
next.url,
'/hooks/use-cookies/client'
)
expect(res.status).toBe(500)
expect(await res.text()).toContain('Internal Server Error')
})
})

describe('previewData function', () => {
// TODO-APP: should enable when implemented
it.skip('should throw an error when imported', async () => {
const res = await fetchViaHTTP(
next.url,
'/hooks/use-preview-data/client'
)
expect(res.status).toBe(500)
expect(await res.text()).toContain('Internal Server Error')
})
})

describe('headers function', () => {
// TODO-APP: should enable when implemented
it.skip('should throw an error when imported', async () => {
const res = await fetchViaHTTP(
next.url,
'/hooks/use-headers/client'
)
expect(res.status).toBe(500)
expect(await res.text()).toContain('Internal Server Error')
})
})

describe('usePathname', () => {
it('should have the correct pathname', async () => {
const html = await renderViaHTTP(next.url, '/hooks/use-pathname')
Expand Down

0 comments on commit 0a97d07

Please sign in to comment.