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

fix: prevent duplicate manifest generation #1070

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Conversation

wattanx
Copy link
Collaborator

@wattanx wattanx commented Jan 16, 2024

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

I want to avoid duplicate generation of client.manifest.
In the case of a webpack production build, the manifest is generated in the following two places:

if (opts.nitro && !nuxt.options.dev && !nuxt.options._prepare) {
await generateWebpackBuildManifest()
}

nuxt.hook('build:compiled', async ({ name }) => {
if (nuxt.options._prepare) { return }
if (name === 'server') {
const jsServerEntry = resolve(nuxt.options.buildDir, 'dist/server/server.js')
await fsp.writeFile(jsServerEntry.replace(/.js$/, '.cjs'), 'module.exports = require("./server.js")', 'utf8')
await fsp.writeFile(jsServerEntry.replace(/.js$/, '.mjs'), 'export { default } from "./server.cjs"', 'utf8')
} else if (name === 'client') {
const manifest = await fsp.readFile(resolve(nuxt.options.buildDir, 'dist/server/client.manifest.json'), 'utf8')
await fsp.writeFile(resolve(nuxt.options.buildDir, 'dist/server/client.manifest.mjs'), 'export default ' + JSON.stringify(normalizeWebpackManifest(JSON.parse(manifest)), null, 2), 'utf8')
}
})

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@wattanx wattanx requested a review from danielroe January 17, 2024 13:27
@wattanx
Copy link
Collaborator Author

wattanx commented Feb 4, 2024

This PR seems likely to solve the strange issue as well.
In a very large project, we know that this can cause the manifest to break as follows due to its impact.
(It's difficult to create a reproduction because this only occurs in large projects...)

[nitro 18:53:28]  ERROR  RollupError: Unterminated string constant (Note that you need plugins to import files that are not JavaScript)


138909:     "file": "",
138910:     "prefetch": true,
138911:     mports": [
                  ^
138912:       "_4b157d7.js",
138913:       "_c3032ed.js",

Using patch-package, I confirmed that no errors occurred.

@wattanx
Copy link
Collaborator Author

wattanx commented Feb 5, 2024

@danielroe
Due to the issues mentioned above, I am considering including it in 3.1.0. Could you please check if there are any problems?

@@ -144,8 +144,8 @@ export async function generateDevSSRManifest (ctx: ViteBuildContext, css: string

export async function writeClientManifest (clientManifest: any, buildDir: string) {
const clientManifestJSON = JSON.stringify(clientManifest, null, 2)
await fse.writeFile(resolve(buildDir, 'dist/server/client.manifest.json'), clientManifestJSON, 'utf-8')
await fse.writeFile(resolve(buildDir, 'dist/server/client.manifest.mjs'), `export default ${clientManifestJSON}`, 'utf-8')
await fse.writeFile(resolve(buildDir, 'dist/server/client.manifest.json'), clientManifestJSON, 'utf8')
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe this change is needed.

@danielroe danielroe merged commit e299e14 into main Feb 5, 2024
33 checks passed
@danielroe danielroe deleted the fix/fix-generate-manifest branch February 5, 2024 13:49
@wattanx
Copy link
Collaborator Author

wattanx commented Feb 5, 2024

Thank you ❀️

@github-actions github-actions bot mentioned this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants