[Feature Request] Production ready NextJS integration that scales #97
Comments
|
Let me make this easy for maintainers.. |
|
first of all @milanbgd011 sorry for the delay in getting back to you, I’ve been off for holidays. I really like the suggestions! it is SUPER appreciated. but there are some design choices here which probably warrant deeper discussion, and theres the fact that it will be hard to maintain over time. i think if you’re interested in setting it up as a repo in your own account we’d be happy to link to you and do a one-off review. thoughts? |
|
Thank you @sw-yx for the reply. Did not want to overload you with more work, that's all. Lets dive into the problem thru this issue, then I will create the repo demonstrating the solution. We just need to agree on basic rules that make sense to both you and me first. I did another iteration on the initial suggestion, changed structure is below. If we want to make Temporal household name for what it does and does it well, we first need to approach the developers in the best possible way so that they can quickly understand the system, easily integrate into their own projects and start playing around with it to be familiar. Then, they will start pushing their product managers to make the switch once they are familiar and confident in the product. Once those companies start to reach bigger workloads they will come to you for support and this is where Temporal will make money (plus the Temporal Cloud soon). Samples provided could be named "simplified architecture for smaller projects" ("use this repo to create clear mental model of Temporal, then organize file structure as you wish"), so devs are aware this is not good scenario for large projects - but just a shortcut to get all up and running allowing devs to be familiar with the techology. 95% of devs drop Temporal because it needs too much time to figure all the details and create clear and easy mental model in their head - so they reach for some simpler but much less powerfull solutions just to do the work and continue with their lives (sometimes they just do not have time available to tackle this with so much time). Since you are first principles based company, we can analyse this all thru Elon Musks's 5 stages of improvement:
|
small number of files/folders yes, but we are not extreme about it, we often offer files for code quality, eg
some users may not want this, but sounds ok for this sample!
what is queue0? why is it here in the filepath? you didnt explain why it is necessary to expose queues in the api route
i dont think nextjs would support workers running as API routes.. but maybe this is not a problem, need ot see your implementation to understand what you are trying to do
can we combine all these into one file? why so many different files, i thought you wanted absolute minimum
this sounds like a GREAT idea
no this is not fitting the vision of the samples, we expect our users to modify the examples heavily and so investing in tooling for them to redownload our README makes no sense for step 5 Automate instead of doing crude overwrite, we should do the "Create React App" approach and encapsulate this in a dependency that we maintain. anyway, we dont have to get to step 5 yet. |
|
Hardest part in programming is creating clear mental model in your head how something works.
Implementation should be based on clean NextJS install with
Goal is here to wire all up with as little as possible number of files/folders so that devs can use it just for familiarization with how Temporal works (individual files for Events seems clearer at least for me). Make even beginner programmers to be able to use it out of the box, so it is ridicilously easy to test it - so not a single dev gives up. I am experienced full-stack dev, but it took me some time to connect arrange all into usable form. React is great, but until NextJS came into picture creating on your own full webpack solution for production was very hard.
Queue0 is example queue that will be linked to "One Click Buy" example page. User will create more queues. Maybe it can be renamed to "DefaultQueue" instead to make things more clear.
This is outside the /pages directory, so it is not a route like a page or API. This is independent TS file that will be run with ie.
Because the code is verbose and if we put all in single file it will be harder to work with. Second is that we loose visiblity of what actions are available and third is that we loose direct pairing with
Cool.
I agree. All updates can be done manually from main repo.
I agree on this. |
|
ok i think we are agreed on these points then! |
|
Thank you for your feedback. Will create PR when I catch a bit of time, so you can test it locally and provide more suggestions for the final solution. |
|
I need more feedback before PR. Am I wrong anywhere? What is not intuitive? What can be changed? Here is my final proposal for NextJS/Temporal integration, by providing the same example in different form as https://github.com/temporalio/samples-typescript/tree/main/nextjs-ecommerce-oneclick already defined one. Naming rules (imporant to clarify that first)
File structure
The workflow file structure (explanations below the code)Goal here is to have full definition of all variables in single exported /* eslint-disable no-void */
/* eslint-disable no-return-assign */
/* eslint-disable no-return-await */
// eslint-disable-next-line
import * as wf from '@temporalio/workflow';
import type * as activities from '../../_activities/_all';
// DEFINITION
export const d = {
taskQueue: 'queue0',
namespace: 'default',
activity: wf.proxyActivities<typeof activities>({
startToCloseTimeout: '1 minute',
}),
query: {},
queryEvent: {
GETSTATE: 'GETSTATE',
},
signal: {},
signalEvent: {
CANCEL: 'CANCEL',
},
state: {
IS_PENDING: 'IS_PENDING',
IS_CONFIRMED: 'IS_CONFIRMED',
IS_CANCELLED: 'IS_CANCELLED',
},
};
d.query[d.queryEvent.GETSTATE] = wf.defineQuery(d.queryEvent.GETSTATE);
d.signal[d.signalEvent.CANCEL] = wf.defineSignal(d.signalEvent.CANCEL);
// WORKFLOW
export async function _oneClickBuy(props) {
const { itemId } = props || {};
// define context
const itemToBuy = itemId;
let purchaseState = d.state.IS_PENDING;
// wire up handlers (aka. event listeners)
wf.setHandler(d.signal[d.signalEvent.CANCEL], () => void (purchaseState = d.state.IS_CANCELLED)); // return nothing, just change context
wf.setHandler(d.query[d.queryEvent.GETSTATE], () => purchaseState); // return data, dont change context
// workflow logic
if (await wf.condition(() => purchaseState === d.state.IS_CANCELLED, '5s')) {
return await d.activity.cancelPurchase(itemToBuy);
}
purchaseState = d.state.IS_CONFIRMED;
return await d.activity.startPurchase(itemToBuy);
}Details explaining choices for workflow file organization
import { NextApiRequest, NextApiResponse } from 'next';
export default async (req: NextApiRequest, res: NextApiResponse): Promise<any> => {
const { slug } = req.query || {};
const [queue, workflow, event] = slug ? (Array.isArray(slug) ? slug : [slug]) : [];
const DynamicAPILoader = await import(`@/__temporal/${queue}/${workflow}/${event}.ts`);
if (DynamicAPILoader.default) {
const [ERRreturnJSON, returnJSON] = await DynamicAPILoader.default({ req, res })
.then((r: any) => [null, r])
.catch((e: any) => [e]);
if (ERRreturnJSON) {
return res.status(500).json({ errors: [{ error: ERRreturnJSON }] });
}
return res.status(200).json(returnJSON);
}
return res.status(500).json({
errors: [
{
error: 'No API endpoint!',
},
],
});
};PS. Congrats Temporal on huge round! So much money for such small team! Temporal will crush everything now, just matter of time. 🚀🚀🚀🚀🚀 |
|
thanks for joining us for the ride! this |
|
Thanks for the feedback!
Thanks for the link.
Try building a workflow using Will wait for couple of days for any additional info/comments to appear. No rush, can create PR in 10 minutes, no problem. |
|
Hi @milanbgd011, I was interested to read the most recent iteration. Thoughts:
|
|
Hi @lorensr thank you for very insightful info. This is type of info I needed. Thank you @sw-yx for pinging @lorensr on this, got really good feedback here! 1. FILE REORGANIZATION PROPOSALI moved files around quickly (just reorder, no coding in there) just to see if I understood you right, can you confirm that this is what you were suggesting? (implementation is not important for now, just the organization of the files)
2. UNIVERSAL API ENDPOINT REFACTORIncorporating the new folder structure and info you provided, maybe this is the best pattern. This way we can put same workflow to different queues and namespaces easy, all is decoupled:
3. WORKFLOW CODE REFACTORORIGINAL VERSION (NEXTJS EXAMPLE)This works, but seems very messy and unreadable to me at least. Very hard to write without making errors because of writing strings manually everywhere. Not ideal. import * as wf from '@temporalio/workflow';
// // Only import the activity types
import type * as activities from './activities';
const { checkoutItem, canceledPurchase } = wf.proxyActivities<typeof activities>({
startToCloseTimeout: '1 minute',
});
type PurchaseState = 'PURCHASE_PENDING' | 'PURCHASE_CONFIRMED' | 'PURCHASE_CANCELED';
export const cancelPurchase = wf.defineSignal('cancelPurchase');
export const purchaseStateQuery = wf.defineQuery<PurchaseState>('purchaseState');
// @@@SNIPSTART typescript-oneclick-buy
export async function OneClickBuy(itemId: string) {
const itemToBuy = itemId;
let purchaseState: PurchaseState = 'PURCHASE_PENDING';
wf.setHandler(cancelPurchase, () => void (purchaseState = 'PURCHASE_CANCELED'));
wf.setHandler(purchaseStateQuery, () => purchaseState);
if (await wf.condition(() => purchaseState === 'PURCHASE_CANCELED', '5s')) {
return await canceledPurchase(itemToBuy);
} else {
purchaseState = 'PURCHASE_CONFIRMED';
return await checkoutItem(itemToBuy);
}
}REFACTORED VERSION PROPOSALThis seems to me much easier to read and figure out what is going on, plus not a single string needs to be written. All is automatically suggested and type safe 100%. Please notice the change from import * as wf from '@temporalio/workflow';
import type * as activities from '../../activities/_all';
// DEFINITION
const states = ['IS_PENDING', 'IS_COMPLETED', 'IS_CANCELLED'] as const;
const queries = ['GETSTATE'] as const;
const signals = ['CANCEL'] as const;
export const d: dI = definitionInit(states, queries, signals);
// WORKFLOW
export async function oneClickBuy(props) {
// context
const itemToBuy = props.itemId;
let purchaseState = d.state.IS_PENDING;
// listeners
wf.setHandler(d.signal.CANCEL.handler, () => void (purchaseState = d.state.IS_CANCELLED)); // return nothing, just change context
wf.setHandler(d.query.GETSTATE.handler, () => purchaseState); // return data, dont change context
// workflow
if (await wf.condition(() => purchaseState === d.state.IS_CANCELLED, '5s')) {
return await d.activity.cancelPurchase(itemToBuy);
}
purchaseState = d.state.IS_CONFIRMED;
return await d.activity.startPurchase(itemToBuy);
}This creates fully typed definition for
If you put
HELPER FUNCTIONS FOR "REFACTORED VERSION"function definitionInit(statesInput, queriesInput, signalsInput) {
return {
activity: wf.proxyActivities<typeof activities>({ startToCloseTimeout: '1 minute' }),
states: statesInput.reduce((acc, key) => ({ ...acc, [key]: key }), {}),
queries: queriesInput.reduce((acc, key) => ({ ...acc, key, query: d.query[key] }), {}),
signals: signalsInput.map((acc, key) => ({ ...acc, key, handler: d.signal[key] }), {}),
};
}
type dI = {
activity: typeof activities;
states: Record<typeof states[number], typeof states[number]>;
query: Record<typeof queries[number], { key: string; handler: () => any }>;
signal: Record<typeof signals[number], { key: string; handler: () => any }>;
};PS. Concerning the FE accessing all. We can expose all routes to the FE, but require specific HASH to be passed in order for it to trigger it, thus preventing the FE to execute something he has no right to do, so this is not hard to do, will think about it last. This example should not think about security at all, this is "figure out Temporal and create mental model" example, that is all. Then it will be pushed to more custom solution from there. PPS. I know this solution will have rough edges somewhere and will not cover all scenarios, but lets push this to its limits, maybe it will be good starting point for some better solution that someone else will suggest. Goal here is to give the best possible starting point for familiarizing with Temporal. Then user will move files around, customize and create fully custom solution for them. It is important to know what is goal here. |
|
Some workflows don't have states or queries or signals, so I'd make a single object param. Also prefer verbs starting function names. I'd say export const d = describeWorkflow({ states, queries, signals });Could also have a Don't need |
|
Thank you @lorensr for the feedback! Means a lot. I refactored all as you suggested, seems like all is incorporated into the code now. Autocomplete works fully, it is very fast to create workflow when all is typed 100%. Tested, all works flawlessly. REFACTORED FILE STRUCTURECreated
REFACTORED WORKFLOWNeeded to add params to PS. When I remove comments, code has only 19 lines, exactly the same as original workflow for nextjs-oneclick example. import * as wf from '@temporalio/workflow';
import { describeWorkflow, dI } from '../../helpers';
import type * as activities from '../../activities/_all';
// DEFINITION
const states = ['IS_PENDING', 'IS_COMPLETED', 'IS_CANCELLED'] as const;
const queries = ['GETSTATE'] as const;
const signals = ['CANCEL'] as const;
export const d: dI<typeof activities, typeof states[number], typeof queries[number], typeof signals[number]> =
describeWorkflow({ states, queries, signals });
// WORKFLOW
export async function oneClickBuy(props) {
// LISTENERS
wf.setHandler(d.signal.CANCEL, () => void (stateCurrent = d.state.IS_CANCELLED));
wf.setHandler(d.query.GETSTATE, () => stateCurrent);
// CONTEXT
let stateCurrent: typeof states[number] = d.state.IS_PENDING; // initial state
const itemId: string = props.itemId;
// WORKFLOW LOGIC
if (await wf.condition(() => stateCurrent === d.state.IS_CANCELLED, '5s')) {
return await d.activity.cancelPurchase({ itemId });
}
stateCurrent = d.state.IS_COMPLETED;
return await d.activity.checkoutItem({ itemId });
}REFACTORED HELPER FUNCTION (./helpers.ts)Type // @ts-nocheck
// eslint-disable-next-line
import * as wf from '@temporalio/workflow';
// eslint-disable-next-line
import { SignalDefinition, QueryDefinition } from '@temporalio/common';
export type dI<A, B, C, D> = {
activity: A;
state: Record<B, B>;
query: Record<C, QueryDefinition<B>>;
signal: Record<D, SignalDefinition>;
};
export function describeWorkflow({ states, queries, signals }): any {
return {
activity: wf.proxyActivities({ startToCloseTimeout: '1 minute' }),
state: states.reduce((acc, key) => ({ ...acc, [key]: key }), {}),
query: queries.reduce((acc, key) => ({ ...acc, [key]: wf.defineQuery(key) }), {}),
signal: signals.reduce((acc, key) => ({ ...acc, [key]: wf.defineSignal(key) }), {}),
};
}Needed to add
REFACTORED UNIFIED API ENDPOINTThis is what I saw looking at the code:
That being said, I will alter the proposal for unified API, putting namespace first in the URL, then queue:
|
|
I added workflow versioning and sinks in this more advanced example, we can make separate full example with all features integrated. On one side we have "simple" workflow and on the other "all included" workflow, so user can add/delete parts needed. Seems like reasonable thing to have. Let me know, when you have some time, what can be polished, so I can create PR for this finally. import * as wf from '@temporalio/workflow';
import { describeWorkflow, dI } from '../../helpers';
import type * as activities from '../../activities/_all';
import { logger } from '../../sinks';
// DEFINITION
const states = ['IS_PENDING', 'IS_COMPLETED', 'IS_CANCELLED'] as const;
const queries = ['GETSTATE'] as const;
const signals = ['CANCEL'] as const;
const workflowVersion = {
'Initial workflow': 'v1', // initial workflow
'Increase timeout to 15sec': 'v2', // increase timeout for buy: 5 > 15 seconds
};
export const d: dI<typeof activities, typeof states[number], typeof queries[number], typeof signals[number]> =
describeWorkflow({ states, queries, signals });
// WORKFLOW
export async function oneClickBuy(props) {
// LISTENERS
wf.setHandler(d.signal.CANCEL, () => void (stateCurrent = d.state.IS_CANCELLED));
wf.setHandler(d.query.GETSTATE, () => stateCurrent);
// CONTEXT
let stateCurrent: typeof states[number] = d.state.IS_PENDING; // initial state
const { itemId } = props || {};
// WORKFLOW
// --- v1 only (initial)
if (await wf.condition(() => stateCurrent === d.state.IS_CANCELLED, '5s')) {
logger.info(`Log to worker console: Cancelled the purchase! Status: ${stateCurrent}`); // for debugging via console or more..
return await d.activity.cancelPurchase({ itemId });
}
// // --- v1/v2 running same time (value of patch will be "v2", simple version and we see desc in code)
// if (wf.patched(workflowVersion['Increase timeout to 15sec'])) {
// if (await wf.condition(() => stateCurrent === d.state.IS_CANCELLED, '15s')) {
// return await d.activity.cancelPurchase(itemToBuy);
// }
// } else if (await wf.condition(() => stateCurrent === d.state.IS_CANCELLED, '5s')) {
// return await d.activity.cancelPurchase(itemToBuy);
// }
// // --- only v2, with deprecate patch
// wf.deprecatePatch(workflowVersion['Increase timeout to 15sec']);
// if (await wf.condition(() => stateCurrent === d.state.IS_CANCELLED, '15s')) {
// return await d.activity.cancelPurchase(itemToBuy);
// }
// // --- only v2
// if (await wf.condition(() => stateCurrent === d.state.IS_CANCELLED, '15s')) {
// return await d.activity.cancelPurchase(itemToBuy);
// }
stateCurrent = d.state.IS_COMPLETED;
logger.info(`Log to worker console: Completed the purchase! Status: ${stateCurrent}`); // for debugging via console or more..
return await d.activity.checkoutItem({ itemId });
}Here is the /sinks.ts file // eslint-disable-next-line
import * as wf from '@temporalio/workflow';
export interface LoggerSinks extends wf.Sinks {
logger: {
error(message: string): void;
info(message: string, data?: unknown): void;
};
}
export const { logger } = wf.proxySinks<LoggerSinks>();
|
|
Can we get export const d = describeWorkflow({ states, queries, signals });If not, then what about something like this: export const d = describeWorkflow<{activities, states, queries, signals}>({ states, queries, signals });(Trying to get it shorter than current) Nice version description-in-code! Can you host the code in your own repo and submit a PR to add it to
(We're committed to keeping the samples in this repo up to date, and I want to protect the maintainer team time for working on the SDK) |
|
and I'd be happy to do a blogpost/video chat with you to walk through some of these best practices that you've highlighted! |
Is your feature request related to a problem? Please describe.
First, to clarify. All examples in this repo are perfect for what they were created to demonstrate, that is most minimalistic implementation of specific feature and is very high value for us all. I am just proposing that we add one more advanced example that will make easier for developers to jump into Temporal fully configured and just start using it. Many developers just give up because setting up stuff for NextJS/Temporal is not within reach of all devs out there.
Figuring out state machines is hard. Figuring out that Temporal workflow instance is a "living statemachine" inside Temporal server is hard. Figuring out Temporal is hard. Figuring out how to integrate Temporal into NextJS properly is hard. Then figuring out how to compile TS to JS so that independent worker can use it is another step to do. Your brain will melt during your voyage thru the matter.
Describe the solution you'd like
Add Temporal integration based on existing "One Click Buy" but one that allows user to easily add more workflows and queues without brain damage during the process.
Additional context
Steps to create tight integration with NextJS that scales:
use this example as starting point https://github.com/temporalio/samples-typescript/tree/main/nextjs-ecommerce-oneclick (then just rewire 2 api calls to use new folder structure proposed below, and all should worK)
look at screenshot to get better understanding of folder structure that is being proposed. all files (almost) live in
./workflowsdirectory only inside nextjs app./workflows/- all files for Temporal are only here (except single dynamic API endpoint that maps to actions)./tsconfig.json- typescript compiler config that will be used to convert .ts to .js as source code for worker instance./__worker_source__- here we compile .ts to .js and this is folder that will worker directly use to start itselft, pure .js files./queue1/is sample naming for first queue, inside it we have 3 files that will be directly consumed by worker instance./activities- exports all activities for worker to import./workflows- exports all workflows for worker to import./worker- actual code that represents worker instance (the file that will be run as worker)./oneClickBuy/- folders are only used for defining workflows that contain this file structure./_workflow-1.ts- first version of workflow definition./_workflow-2.ts- second revision of workflow definition./_workflow-current.ts- current revision of workflow definition (we are creating revisions of the same workflow during time, we need to keep all previous versions of workflow definitions that still have at least 1 instance running inside temporal server, so that we can compare old and new workflow definition and make change in latest current so that older workflow instances can work. if no instances of previous revisions exists, they are deleted or after implementation of new)./CANCEL_act.ts- code that will worker execute against external internet resources if needed./CANCEL_api.ts- processes request that we got from /api/workflows/queue1/oneClickBuy/CANCEL request./GETSTATE_api.ts- processes request that we got from /api/workflows/queue1/oneClickBuy/GETSTATE request./START_act.ts- code that will worker execute against external internet resources if needed./START_api.ts- processes request that we got from /api/workflows/queue1/oneClickBuy/START requestAnyone familiar with state machines (ie. Xstate) and used it in production systems both front and backend or has used Redux as concept to simplify state management, will be able to recognize pattern here.
With UPPERCASE letters we write "events" that interact with our workflow instance. The workflow instance template is in _workflow-current.ts file and START_api.ts will be responsible to create instance of that workflow in temporal server.
Now reasoning about the code is much much easier. We define workflow and we define events that interact with the instance of workflow that lives in Temporal server, very easy. Each interaction with any queue / workflow / event is as easy as triggering
/api/workflows/queue1/oneClickBuy/STARTand that is it, it can be consumed from nextjs app or from external system.docker-compose -f ./docker-compose-temporal.yml up -dto start temporal server on local machinenpm run temporaldev- to start typescript compiler that will read ./workflows and create .jsnpm run temporalworkerqueue1- to start instance of workernpm run dev- to run nextjs dev serverlocalhost:3000and goto page to click on products to orderhttp://localhost:8088Temporal web interface and check if all works./package.json in scripts segment
/pages/api/workflows/[[...slug]].ts
./workflows/tsconfig.json
./workflows/workflows.ts
./workflows/activities.ts
./workflows/worker.ts
./workflows/queue1/_workflow-current.ts
./workflows/queue1/CANCEL_act.ts
./workflows/queue1/CANCEL_api.ts
./workflows/queue1/GETSTATE_api.ts
./workflows/queue1/CANCEL_act.ts
./workflows/queue1/CANCEL_act.ts
ADDITIONAL FILES NEEDED
./docker-compose-temporal.yml
./.docker-compose/temporal-dev-es.yaml
./docker-compose/temporal-dev.yaml
The text was updated successfully, but these errors were encountered: