Skip to content

Commit

Permalink
Fix export from and native modules in server component (#36072)
Browse files Browse the repository at this point in the history
This PR fixes a bunch of bugs and it now supports:
- Importing a client component from a nested server component (a.server → b.server → c.client).
- The `export from` syntax in server component (`export { default } from './a.server'`)
- Native modules in server components (currently broken)

## Buga

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have 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 helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
  • Loading branch information
shuding committed Apr 13, 2022
1 parent 20600ad commit 19b625e
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 22 deletions.
78 changes: 57 additions & 21 deletions packages/next/build/webpack/loaders/next-flight-server-loader.ts
@@ -1,3 +1,5 @@
import { builtinModules } from 'module'

import { parse } from '../../swc'
import { buildExports } from './utils'

Expand All @@ -24,10 +26,8 @@ export const createServerComponentFilter = (extensions: string[]) => {
return (importSource: string) => regex.test(importSource)
}

function createFlightServerRequest(request: string, extensions: string[]) {
return `next-flight-server-loader?${JSON.stringify({
extensions,
})}!${request}`
function createFlightServerRequest(request: string, options: object) {
return `next-flight-server-loader?${JSON.stringify(options)}!${request}`
}

function hasFlightLoader(request: string, type: 'client' | 'server') {
Expand Down Expand Up @@ -67,16 +67,47 @@ async function parseModuleInfo({
let imports = []
let __N_SSP = false
let pageRuntime = null
let isBuiltinModule
let isNodeModuleImport

const isEsm = type === 'Module'

async function getModuleType(path: string) {
const isBuiltinModule_ = builtinModules.includes(path)
const resolvedPath = isBuiltinModule_ ? path : await resolver(path)

const isNodeModuleImport_ = resolvedPath.includes('/node_modules/')

return [isBuiltinModule_, isNodeModuleImport_] as const
}

function addClientImport(path: string) {
if (isServerComponent(path) || hasFlightLoader(path, 'server')) {
// If it's a server component, we recursively import its dependencies.
imports.push(path)
} else if (isClientComponent(path)) {
// Client component.
imports.push(path)
} else {
// Shared component.
imports.push(
createFlightServerRequest(path, {
extensions,
client: 1,
})
)
}
}

for (let i = 0; i < body.length; i++) {
const node = body[i]
switch (node.type) {
case 'ImportDeclaration':
const importSource = node.source.value
const resolvedPath = await resolver(importSource)
const isNodeModuleImport = resolvedPath.includes('/node_modules/')

;[isBuiltinModule, isNodeModuleImport] = await getModuleType(
importSource
)

// matching node_module package but excluding react cores since react is required to be shared
const isReactImports = [
Expand Down Expand Up @@ -104,9 +135,10 @@ async function parseModuleInfo({
imports.push(importSource)
} else {
// A shared component. It should be handled as a server component.
const serverImportSource = isReactImports
? importSource
: createFlightServerRequest(importSource, extensions)
const serverImportSource =
isReactImports || isBuiltinModule
? importSource
: createFlightServerRequest(importSource, { extensions })
transformedSource += importDeclarations
transformedSource += JSON.stringify(serverImportSource)

Expand All @@ -116,18 +148,10 @@ async function parseModuleInfo({
}
}
} else {
// For the client compilation, we skip all modules imports but
// always keep client/shared components in the bundle. All client components
// have to be imported from either server or client components.
if (
isServerComponent(importSource) ||
hasFlightLoader(importSource, 'server') ||
// TODO: support handling RSC components from node_modules
isNodeModuleImport
) {
continue
}
imports.push(importSource)
// For now we assume there is no .client.js inside node_modules.
// TODO: properly handle this.
if (isNodeModuleImport || isBuiltinModule) continue
addClientImport(importSource)
}

lastIndex = node.source.span.end
Expand Down Expand Up @@ -158,6 +182,18 @@ async function parseModuleInfo({
}
}
break
case 'ExportNamedDeclaration':
if (isClientCompilation) {
if (node.source) {
// export { ... } from '...'
const path = node.source.value
;[isBuiltinModule, isNodeModuleImport] = await getModuleType(path)
if (!isBuiltinModule && !isNodeModuleImport) {
addClientImport(path)
}
}
}
break
default:
break
}
Expand Down
Expand Up @@ -72,7 +72,7 @@ export class FlightManifestPlugin {
// TODO: Hook into deps instead of the target module.
// That way we know by the type of dep whether to include.
// It also resolves conflicts when the same module is in multiple chunks.
if (!isClientComponent(resource)) {
if (!resource || !isClientComponent(resource)) {
return
}
const moduleExports: any = manifest[resource] || {}
Expand Down
@@ -0,0 +1,16 @@
import fs from 'fs'

import Foo from '../components/foo.client'

export default function Page() {
return (
<>
<h1>fs: {typeof fs.readFile}</h1>
<Foo />
</>
)
}

export const config = {
runtime: 'nodejs',
}
@@ -0,0 +1 @@
export { default } from './css-modules.server'
15 changes: 15 additions & 0 deletions test/integration/react-streaming-and-server-components/test/rsc.js
Expand Up @@ -220,6 +220,21 @@ export default function (context, { runtime, env }) {
expect(hydratedContent).toContain('Export All: one, two, two')
})

it('should support native modules in server component', async () => {
const html = await renderViaHTTP(context.appPort, '/native-module')
const content = getNodeBySelector(html, '#__next').text()

expect(content).toContain('fs: function')
expect(content).toContain('foo.client')
})

it('should support the re-export syntax in server component', async () => {
const html = await renderViaHTTP(context.appPort, '/re-export')
const content = getNodeBySelector(html, '#__next').text()

expect(content).toContain('This should be in red')
})

it('should handle 404 requests and missing routes correctly', async () => {
const id = '#text'
const content = 'custom-404-page'
Expand Down

0 comments on commit 19b625e

Please sign in to comment.