feat: move publish to server-side#2338
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| envVars: {}, | ||
| }); | ||
|
|
||
| // TODO: handle response |
There was a problem hiding this comment.
The publish() function has a TODO comment regarding response handling. Consider checking and handling the response (errors/success) from publish API instead of always showing a success toast.
|
|
||
| async unpublish(projectId: string, urls: string[]): Promise<PublishResponse> { | ||
| try { | ||
| const success = await deployFreestyle(PublishType.UNPUBLISH, projectId, {}, urls); |
There was a problem hiding this comment.
In the unpublish method, deployFreestyle is called with incorrect arguments. It’s missing the DB connection as the first parameter, and the config object is not structured correctly. Adjust the call to match the deployFreestyle signature.
| const success = await deployFreestyle(PublishType.UNPUBLISH, projectId, {}, urls); | |
| const success = await deployFreestyle(this.session.db, PublishType.UNPUBLISH, projectId, {}, { domains: urls }); |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| const response = await publishManager.publish({ | ||
| type, | ||
| projectId, | ||
| buildScript, | ||
| buildFlags, | ||
| envVars, | ||
| }); | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
The publish mutation doesn't return a response to the client. After calling publishManager.publish(), the response should be returned to complete the API contract. Add:
return response;after line 59 to ensure the client receives the publish operation result.
| const response = await publishManager.publish({ | |
| type, | |
| projectId, | |
| buildScript, | |
| buildFlags, | |
| envVars, | |
| }); | |
| const response = await publishManager.publish({ | |
| type, | |
| projectId, | |
| buildScript, | |
| buildFlags, | |
| envVars, | |
| }); | |
| return response; | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
|
|
||
| async unpublish(projectId: string, urls: string[]): Promise<PublishResponse> { | ||
| try { | ||
| const success = await deployFreestyle(PublishType.UNPUBLISH, projectId, {}, urls); |
There was a problem hiding this comment.
The deployFreestyle function call is missing required parameters. The correct function signature requires the database connection as the first parameter:
const success = await deployFreestyle(
this.session.db,
PublishType.UNPUBLISH,
projectId,
{},
{
domains: urls,
entrypoint: 'server.js'
}
);This matches the implementation in the deploy.ts file where the first parameter is the database connection.
| const success = await deployFreestyle(PublishType.UNPUBLISH, projectId, {}, urls); | |
| const success = await deployFreestyle( | |
| this.session.db, | |
| PublishType.UNPUBLISH, | |
| projectId, | |
| {}, | |
| { domains: urls, entrypoint: 'server.js' } | |
| ); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| const response = await publishManager.publish({ | ||
| type, | ||
| projectId, | ||
| buildScript, | ||
| buildFlags, | ||
| envVars, | ||
| }); |
There was a problem hiding this comment.
The publishManager.publish() call is missing several required parameters according to the function signature in the manager.ts file. Please add the following parameters to complete the call:
const response = await publishManager.publish({
type,
projectId,
buildScript,
buildFlags,
envVars,
urls: foundPreviewDomains.map(d => d.fullDomain),
skipBadge: false,
skipBuild: false
});Without these parameters, the publish operation will fail when it attempts to access them.
| const response = await publishManager.publish({ | |
| type, | |
| projectId, | |
| buildScript, | |
| buildFlags, | |
| envVars, | |
| }); | |
| const response = await publishManager.publish({ | |
| type, | |
| projectId, | |
| buildScript, | |
| buildFlags, | |
| envVars, | |
| urls: foundPreviewDomains.map(d => d.fullDomain), | |
| skipBadge: false, | |
| skipBuild: false | |
| }); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| } | ||
|
|
||
| // Deploy to Freestyle | ||
| deployFreestyle({ |
There was a problem hiding this comment.
The deployFreestyle call isn’t awaited. Use 'await' so that any deployment errors are properly handled before the procedure completes.
| deployFreestyle({ | |
| await deployFreestyle({ |
| // Run domain check | ||
|
|
||
| // Delete deployment | ||
| deployFreestyle({ |
There was a problem hiding this comment.
The unpublish deployFreestyle call isn’t awaited. Await it to catch asynchronous errors during deletion.
| deployFreestyle({ | |
| await deployFreestyle({ |
| }), | ||
|
|
||
| unpublish: publicProcedure.input(z.object({ | ||
| type: z.nativeEnum(PublishType), |
There was a problem hiding this comment.
The unpublish procedure’s input includes a 'type' field that isn’t used. Consider removing it if unnecessary.
| type: z.nativeEnum(PublishType), |
| const unpublish = async (projectId: string, type: DeploymentType) => { | ||
| const response = await runUnpublish({ | ||
| projectId, | ||
| type: DeploymentType.UNPUBLISH_ALL, |
There was a problem hiding this comment.
The unpublish function always sets the type to DeploymentType.UNPUBLISH_ALL, ignoring the passed type parameter. This seems unintended given downstream calls expect specific unpublish types.
| type: DeploymentType.UNPUBLISH_ALL, | |
| type, |
| deployFreestyle({ | ||
| files, | ||
| urls: deploymentUrls, | ||
| envVars, | ||
| }); |
There was a problem hiding this comment.
The deployFreestyle() function returns a Promise but is being called without await. This could lead to the function continuing execution before deployment is complete, potentially causing the cleanup to start prematurely. Consider modifying to:
const result = await deployFreestyle({
files,
urls: deploymentUrls,
envVars,
});
This ensures the deployment completes before moving to cleanup steps and would also allow checking the result for success/failure.
| deployFreestyle({ | |
| files, | |
| urls: deploymentUrls, | |
| envVars, | |
| }); | |
| const result = await deployFreestyle({ | |
| files, | |
| urls: deploymentUrls, | |
| envVars, | |
| }); | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Move publish functionality to server-side with new deployment architecture and API.
publishRouterinrouters/publish/index.ts.DeploymentTypeandDeploymentStatusenums inmodels/hosting/index.ts.helpers.tsandmanager.ts.publishRoutertoapi/root.tsandrouters/index.ts.deploymentRouterinrouters/publish/deployment.tsfor deployment queries and updates.forkBuildSandboxinrouters/publish/fork.tsfor sandbox management.deploymentstable schema inschema/project/deployment.ts.drizzle-ormfor database operations.custom-domain-section.tsx,preview-domain-section.tsx, andtrigger-button.tsx.HostingProvideranduseHostingTypeinstore/hostingfor state management.providers.tsxto includeHostingProvider.This description was created by
for eb7dbdc. You can customize this summary. It will automatically update as commits are pushed.