Ensure setAssetPrefix updates config instance#82160
Conversation
|
|
||
| public setAssetPrefix(prefix?: string): void { | ||
| this.renderOpts.assetPrefix = prefix ? prefix.replace(/\/$/, '') : '' | ||
| this.nextConfig.assetPrefix = prefix ? prefix.replace(/\/$/, '') : '' |
There was a problem hiding this comment.
The setAssetPrefix method now sets nextConfig.assetPrefix instead of renderOpts.assetPrefix, but other parts of the codebase still expect renderOpts.assetPrefix to be available, causing asset prefix to not work correctly.
View Details
Analysis
The change on line 1655 modified setAssetPrefix to update this.nextConfig.assetPrefix instead of this.renderOpts.assetPrefix. However, there are still two locations in the codebase that depend on renderOpts.assetPrefix:
packages/next/src/server/async-storage/work-store.tsline 129:assetPrefix: renderOpts?.assetPrefix || '',packages/next/src/server/post-process.tsline 45:publicPath: \${renderOpts.assetPrefix}/_next/`,`
Both RenderOptsPartial interfaces in render.tsx and app-render/types.ts define assetPrefix?: string as an expected property. When setAssetPrefix is called during server initialization, the asset prefix will be stored in nextConfig but the code expecting it in renderOpts will receive undefined, potentially causing incorrect asset URLs or broken CSS optimization.
Recommendation
The setAssetPrefix method should update both locations to maintain consistency. Change line 1655 to:
public setAssetPrefix(prefix?: string): void {
const assetPrefix = prefix ? prefix.replace(/\/$/, '') : ''
this.nextConfig.assetPrefix = assetPrefix
this.renderOpts.assetPrefix = assetPrefix
}Alternatively, if the intention is to only store it in nextConfig, then the code in work-store.ts and post-process.ts should be updated to read from nextConfig.assetPrefix instead of renderOpts.assetPrefix.
There was a problem hiding this comment.
The above referenced renderOpts.assetPrefix usage is derived from the new handlers interface which is not from the base-server renderOpts instance. This will be cleared up while deleting base-server code.
| ): Promise<void> | ||
|
|
||
| public setAssetPrefix(prefix?: string): void { | ||
| this.renderOpts.assetPrefix = prefix ? prefix.replace(/\/$/, '') : '' |
There was a problem hiding this comment.
We're still using renderOpts.assetPrefix here, do we still need to update the renderOpts.assetPrefix as well?
This ensures we update `nextConfig.assetPrefix` instead of `renderOpts` as we aren't relying on `renderOpts` for these values anymore. Closes: #82150
This ensures we update
nextConfig.assetPrefixinstead ofrenderOptsas we aren't relying onrenderOptsfor these values anymore.Closes: #82150