fix(deploy): move saving the build config to deploy plugins #888
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
| @@ -64,6 +64,7 @@ import { deployCloudflarePlugin } from '../plugins/vite-plugin-deploy-cloudflare | |||
| import { deployDenoPlugin } from '../plugins/vite-plugin-deploy-deno.js'; | |||
| import { deployPartykitPlugin } from '../plugins/vite-plugin-deploy-partykit.js'; | |||
| import { deployAwsLambdaPlugin } from '../plugins/vite-plugin-deploy-aws-lambda.js'; | |||
| import { appendFileSync } from 'node:fs'; | |||
There was a problem hiding this comment.
The other vite deploy fs operations are sync which is why this is not using a promise.
Yeah, I was also curious about it. Good that you confirm. |
| unstable_writeBuildConfig( | ||
| path.join(outDir, WORKER_JS_NAME, DIST_ENTRIES_JS), | ||
| ); |
There was a problem hiding this comment.
I don't like the complexity and exposing the function from build.ts (which doesn't work if you own the plugin outside waku repo.)
Let's simply do this here:
| unstable_writeBuildConfig( | |
| path.join(outDir, WORKER_JS_NAME, DIST_ENTRIES_JS), | |
| ); | |
| appendFileSync( | |
| path.join(outDir, WORKER_JS_NAME, DIST_ENTRIES_JS), | |
| `export const buildData = ${JSON.stringify(platformObject.buildData)};`, | |
| ); |
We will revisit the build process in the future anyway.
There was a problem hiding this comment.
OK. A library of helper functions for deploy authors to use would be nice. The "isNodeCompatible" replacement is another candidate for a helper function:
config.ssr.target = 'webworker';
config.ssr.resolve ||= {};
config.ssr.resolve.conditions ||= [];
config.ssr.resolve.conditions.push('worker');
config.ssr.resolve.externalConditions ||= [];
config.ssr.resolve.externalConditions.push('worker');
|
@dai-shi I attempted to move the updating of entries.js with build data to each deploy plugin. I tested with cloudflare and it works great. Implementation in the other deploy plugins needs testing. |
rmarscher
changed the title
rmarscher
force-pushed
the
fix/deploy-plugin-save-build-config
branch
from
03f4f4f
to
b2c4836
Compare
| delete platformObject.buildOptions.unstable_phase; | ||
|
|
||
| await appendFile( | ||
| distEntriesFile, | ||
| `export const buildData = ${JSON.stringify(platformObject.buildData)};`, | ||
| ); |
There was a problem hiding this comment.
Please keep this because this is required even without deploy plugins.
Only change cloudflare deploy plugin. I don't know the future, but my understanding is only cloudflare has special requirement for the dist folder structure.
There was a problem hiding this comment.
OK... this means the entries.js will be exposed publicly on Cloudflare deployments at /entries.js unless users know to delete it from their dist folder before deploying.
There was a problem hiding this comment.
I can add a note about it in the guide.
There was a problem hiding this comment.
Wouldn't it be the same previously?
I'm fine to add a check:
if (existsSync(distEntriesFile)) {
await appendFile(
distEntriesFile,
`export const buildData = ${JSON.stringify(platformObject.buildData)};`,
);
}There was a problem hiding this comment.
@dai-shi OK great. I amended this PR.
rmarscher
force-pushed
the
fix/deploy-plugin-save-build-config
branch
from
b2c4836
to
8301362
Compare
rmarscher
force-pushed
the
fix/deploy-plugin-save-build-config
branch
from
8301362
to
b34ce5c
Compare
This (along with the previous #874) resolves the issue in #871. The build config was being written to the wrong location for cloudflare pages This PR exports a function from the build module that appends the build config to the given entries file location. It uses a new unstable build config state variable (
unstable_buildConfigSave) to track whether or not the build config has be saved to the entries file. This allows the cloudflare deploy plugin to write the build config into the worker function directory instead of the root.We could merge #879 and then I can edit the base branch to main and continue with this PR. Or however you want to handle it. Thank you!