Skip to content

Commit

Permalink
Make sure polyfills are added for browsers without module support (ve…
Browse files Browse the repository at this point in the history
…rcel#41029)

This PR makes sure that the same polyfills are added in app dir for
browsers that match `nomodule`. Main difference from the polyfills in
pages is that the script tags here cannot have `defer` as all other
scripts will be async by default, and polyfills must be executed before
all of them.

## 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`
- [x] 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)
  • Loading branch information
shuding authored and BowlingX committed Oct 5, 2022
1 parent cb345bc commit a585cf0
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 19 deletions.
Expand Up @@ -17,17 +17,10 @@ export class SubresourceIntegrityPlugin {
stage: webpack.Compilation.PROCESS_ASSETS_STAGE_ADDITIONS,
},
(assets) => {
// Collect all the entrypoint files.
// Collect all the assets.
let files = new Set<string>()
for (const entrypoint of compilation.entrypoints.values()) {
const iterator = entrypoint?.getFiles()
if (!iterator) {
continue
}

for (const file of iterator) {
files.add(file)
}
for (const asset of compilation.getAssets()) {
files.add(asset.name)
}

// For each file, deduped, calculate the file hash.
Expand Down
30 changes: 22 additions & 8 deletions packages/next/server/app-render.tsx
Expand Up @@ -1287,6 +1287,16 @@ export async function renderToHTMLOrFlight(
return flushed
}

const polyfills = buildManifest.polyfillFiles
.filter(
(polyfill) =>
polyfill.endsWith('.js') && !polyfill.endsWith('.module.js')
)
.map((polyfill) => ({
src: `${renderOpts.assetPrefix || ''}/_next/${polyfill}`,
integrity: subresourceIntegrityManifest?.[polyfill],
}))

try {
const renderStream = await renderToInitialStream({
ReactDOMServer,
Expand All @@ -1295,14 +1305,16 @@ export async function renderToHTMLOrFlight(
onError: htmlRendererErrorHandler,
nonce,
// Include hydration scripts in the HTML
bootstrapScripts: subresourceIntegrityManifest
? buildManifest.rootMainFiles.map((src) => ({
src: `${renderOpts.assetPrefix || ''}/_next/` + src,
integrity: subresourceIntegrityManifest[src],
}))
: buildManifest.rootMainFiles.map(
(src) => `${renderOpts.assetPrefix || ''}/_next/` + src
),
bootstrapScripts: [
...(subresourceIntegrityManifest
? buildManifest.rootMainFiles.map((src) => ({
src: `${renderOpts.assetPrefix || ''}/_next/` + src,
integrity: subresourceIntegrityManifest[src],
}))
: buildManifest.rootMainFiles.map(
(src) => `${renderOpts.assetPrefix || ''}/_next/` + src
)),
],
},
})

Expand All @@ -1311,6 +1323,7 @@ export async function renderToHTMLOrFlight(
generateStaticHTML: generateStaticHTML,
flushEffectHandler,
flushEffectsToHead: true,
polyfills,
})
} catch (err: any) {
// TODO-APP: show error overlay in development. `element` should probably be wrapped in AppRouter for this case.
Expand Down Expand Up @@ -1341,6 +1354,7 @@ export async function renderToHTMLOrFlight(
generateStaticHTML: generateStaticHTML,
flushEffectHandler,
flushEffectsToHead: true,
polyfills,
})
}
}
Expand Down
17 changes: 16 additions & 1 deletion packages/next/server/node-web-streams-helper.ts
Expand Up @@ -265,12 +265,14 @@ export async function continueFromInitialStream(
generateStaticHTML,
flushEffectHandler,
flushEffectsToHead,
polyfills,
}: {
suffix?: string
dataStream?: ReadableStream<Uint8Array>
generateStaticHTML: boolean
flushEffectHandler?: () => Promise<string>
flushEffectsToHead: boolean
polyfills?: { src: string; integrity: string | undefined }[]
}
): Promise<ReadableStream<Uint8Array>> {
const closeTag = '</body></html>'
Expand All @@ -289,13 +291,26 @@ export async function continueFromInitialStream(
dataStream ? createInlineDataStream(dataStream) : null,
suffixUnclosed != null ? createSuffixStream(closeTag) : null,
createHeadInjectionTransformStream(async () => {
// Inject polyfills for browsers that don't support modules. It has to be
// blocking here and can't be `defer` because other scripts have `async`.
const polyfillScripts = polyfills
? polyfills
.map(
({ src, integrity }) =>
`<script src="${src}" nomodule=""${
integrity ? ` integrity="${integrity}"` : ''
}></script>`
)
.join('')
: ''

// TODO-APP: Inject flush effects to end of head in app layout rendering, to avoid
// hydration errors. Remove this once it's ready to be handled by react itself.
const flushEffectsContent =
flushEffectHandler && flushEffectsToHead
? await flushEffectHandler()
: ''
return flushEffectsContent
return polyfillScripts + flushEffectsContent
}),
].filter(nonNullable)

Expand Down
7 changes: 7 additions & 0 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -99,6 +99,13 @@ describe('app dir', () => {
expect(html).toContain('hello from dynamic on client')
})

it('should serve polyfills for browsers that do not support modules', async () => {
const html = await renderViaHTTP(next.url, '/dashboard/index')
expect(html).toMatch(
/<script src="\/_next\/static\/chunks\/polyfills(-\w+)?\.js" nomodule="">/
)
})

// TODO-APP: handle css modules fouc in dev
it.skip('should handle css imports in next/dynamic correctly', async () => {
const browser = await webdriver(next.url, '/dashboard/index')
Expand Down

0 comments on commit a585cf0

Please sign in to comment.