Support for Next.JS projects in generated clients #289

DomBlack · 2022-06-24T15:24:59Z

This commit adds support for generated Typescript clients to be
compitable with Next.JS applications with the new --preset nextjs argument.

The existing client was not compiatble as we where using namespaces
to mimic the packages within the Encore application, however namespaces
are somewhat depcreated and not all transpliers support them; the key one
here being Babel (which Next.JS uses under the hood).

When turned on the new --preset nextjs flag, therefore prevents the code generator
from emitting namespaces, and instead it uses the package name as a prefix for
generated types; such as $package_$struct.

To make NextJS development easier, this flag also tells the TypeScript code generator
to generate SWR wrappers for all API's, while maintaining the
promise based functions if direct calls are still required (i.e. to perform actions).

If the original api call was:

const api = new Client()

const rsp: Promise<Data> = api.MySvc.MyAPI("segment", { name: "bob" })

The SWR wrapper will be:

const api = new Client() // this needs to be a singleton for the application instance

const rsp: SWRResponse<Data> = api.MySvc.useMyAPI("segment", { name: "bob" }, { refreshInternval: 1000 })

encore-cla · 2022-06-24T15:25:02Z

All committers have signed the CLA.

This commit adds support for generated Typescript clients to be compitable with [Next.JS](https://nextjs.org/) applications with the new `--nextjs` argument. The existing client was not compiatble as we where using namespaces to mimic the packages within the Encore application, however namespaces are somewhat depcreated and not all transpliers support them; the key one here being Babel (which Next.JS uses under the hood). When turned on the new `--nextjs` flag, therefore prevents the code generator from emitting namespaces, and instead it uses the package name as a prefix for generated types; such as `$package_$struct`. To make NextJS development easier, this flag also tells the TypeScript code generator to generate [SWR](https://swr.vercel.app/) wrappers for all API's, while maintaining the promise based functions if direct calls are still required (i.e. to perform actions). If the original api call was: ``` const api = new Client() const rsp: Promise<Data> = api.MySvc.MyAPI("segment", { name: "bob" }) ``` The SWR wrapper will be: ``` const api = new Client() // this needs to be a singleton for the application instance const rsp: SWRResponse<Data> = api.MySvc.useMyAPI("segment", { name: "bob" }, { refreshInternval: 1000 }) ```

eandre · 2022-06-27T12:44:46Z

This looks great! My only concern is that Next.JS is a moving target, and I can imagine lots of other scenarios where the same code generation options (no namespaces, swr support) are desirable by others who don't necessarily use Next. That would lead to the regrettable situation where if Next.JS changes, we'd be torn between updating the --nextjs flag to match the new behavior (and breaking other users who may still want the old behavior), and introducing a new option to select the new behavior (causing --nextjs to lose its intended meaning).

I think a better solution is to introduce code generation options that map to specific functionality rather than moving targets. So in this case we could have --ts.namespaces=false and --ts.swr=true or something to that effect.

DomBlack · 2022-06-27T12:51:56Z

I think a better solution is to introduce code generation options that map to specific functionality rather than moving targets. So in this case we could have --ts.namespaces=false and --ts.swr=true or something to that effect.

Hmm; what we we do both; --nextjs as a moving target that we keep as a preset for the current target of NextJS, and then specific toggles underneath?

DomBlack · 2022-06-27T14:42:09Z

@eandre; I've updated the CLI options and per your suggestion an introduced a --preset nextjs

eandre · 2022-06-28T21:42:13Z

cli/daemon/run/testdata/echo_client/client.ts

@@ -214,12 +235,16 @@ export namespace echo {

            // Now make the actual call to the API
            const resp = await this.baseClient.callAPI("POST", `/echo.HeadersEcho`, undefined, {headers})
+            try {



With the addition of try { this newline seems out of place. Remove?

eandre · 2022-06-28T21:43:41Z

cli/daemon/run/testdata/echo_client/client.ts

+            try {
+                return await resp.json() as EmptyData
+            } catch (err) {
+                throw new APIError(500, { code: ErrCode.DataLoss, message: "unable to unmarshal the response", details: err })


DataLoss is not meant for this but rather fatal, permanent data loss on the server side. Change to Internal

eandre · 2022-06-28T21:45:25Z

cli/internal/codegen/client.go

@@ -44,7 +45,7 @@ func Detect(path string) (lang Lang, ok bool) {
 }

 // Client generates an API client based on the given app metadata.
-func Client(lang Lang, appSlug string, md *meta.Data) (code []byte, err error) {
+func Client(lang Lang, appSlug string, md *meta.Data, tsOptions *daemonpb.GenClientRequest_TypeScriptOptions) (code []byte, err error) {


I guess making the typescript options part of this signature is intended as a short-term thing? Ideally a nicer interface would be nicer to represent lang+options as a single protobuf message with a oneof

eandre · 2022-06-28T21:47:18Z

cli/internal/codegen/client.go

@@ -54,7 +55,11 @@ func Client(lang Lang, appSlug string, md *meta.Data) (code []byte, err error) {
 	var gen generator
 	switch lang {
 	case LangTypeScript:
-		gen = &typescript{generatorVersion: typescriptGenLatestVersion}
+		if tsOptions == nil {
+			tsOptions = &daemonpb.GenClientRequest_TypeScriptOptions{Namespaces: true}


Instead of doing this here we could make the client always pass in options as it's the thing that's aware of the presets, and the base case of "no preset" is essentially just the default preset

eandre · 2022-06-28T21:48:00Z

cli/internal/codegen/testdata/nextjs_expected_noauth_typescript.ts

+
+
+


Lots of empty newlines here, is that intended?

eandre · 2022-06-28T21:48:20Z

cli/internal/codegen/testdata/nextjs_expected_noauth_typescript.ts

+}
+// CallParameters is the type of the parameters to a method call, but require headers to be a Record type


Should be a newline here

eandre · 2022-06-28T21:48:30Z

cli/internal/codegen/testdata/nextjs_expected_noauth_typescript.ts

+
+

Should be single newline?

eandre · 2022-06-28T21:54:42Z

cli/cmd/encore/gen.go

+					lang = string(codegen.LangTypeScript)
+					tsOptions = &daemonpb.GenClientRequest_TypeScriptOptions{
+						Namespaces: false,
+						Swr:        true,


I'm not sold on coupling swr with next.js. Yes, swr is also developed by the same authors but I believe React Query is increasingly becoming the library of choice for this sort of thing.

What's more, the useBlah helpers are only valid for endpoints that fetch data, not for mutations, since they regularly fire multiple times in the background. Unintentionally calling a stateful, mutating endpoint when not intending to is recipe for disaster, so unless we have a good way of representing which endpoints actually do that and which don't we shouldn't generate such wrappers I don't think.

eandre · 2022-06-28T21:55:07Z

cli/cmd/encore/gen.go

+Support presets are:
+	nextjs: Generates a TypScript client for use within a Next.js application


Suggested change

Support presets are:

nextjs: Generates a TypScript client for use within a Next.js application

Supported presets are:

nextjs: Generates a TypeScript client for use within a Next.js application

eandre · 2022-06-28T21:56:39Z

cli/cmd/encore/gen.go

-			if output == "" && lang == "" {
-				fatal("specify at least one of --output or --lang.")
+			if output == "" && lang == "" && preset == "" {
+				fatal("specify at least one of --output, --lang or --preset.")


I am concerned that this makes for quite the analysis paralysis when trying to decide how to use this command. Could we combine lang and preset somehow to just a single option? We could have --lang=ts choose the default preset and --lang=ts.nextjs (or --lang=ts/nextjs) choose the nextjs preset, or whatever.

melkstam · 2022-08-04T16:00:24Z

We have earlier had compilation errors with namespaces in the generated TS client defined after the class in which they are referenced. However, as of NextJS 12.2.0, this is no longer an issue for us. The generated client compiles and works without any problems. I have not managed to locate the PR that solves this, but have not looked to much at it.

I thought it be good for you to know this since it might change the scope of this PR!

DomBlack · 2022-09-29T12:51:45Z

I'm going to close this PR for now as I'm not sure we need it anymore.

We can re-open it if we needed it agian

DomBlack requested a review from eandre June 24, 2022 15:24

DomBlack enabled auto-merge (squash) June 24, 2022 15:26

DomBlack force-pushed the nextjs-typescript branch from cedb1f8 to f1b9a20 Compare June 24, 2022 16:07

DomBlack force-pushed the nextjs-typescript branch from f1b9a20 to 82361e2 Compare June 24, 2022 17:42

Update old golden test with new TypeScript changes

b4af18a

DomBlack force-pushed the nextjs-typescript branch from e8e1d39 to 3dbe7b6 Compare June 27, 2022 14:44

Regig Next JS support CLI options

15192c6

DomBlack force-pushed the nextjs-typescript branch from 3dbe7b6 to 15192c6 Compare June 28, 2022 07:16

eandre reviewed Jun 28, 2022

View reviewed changes

DomBlack closed this Sep 29, 2022

auto-merge was automatically disabled September 29, 2022 12:51
Pull request was closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Next.JS projects in generated clients #289

Support for Next.JS projects in generated clients #289

DomBlack commented Jun 24, 2022 •

edited

encore-cla bot commented Jun 24, 2022

eandre commented Jun 27, 2022

DomBlack commented Jun 27, 2022

DomBlack commented Jun 27, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022

eandre Jun 28, 2022 •

edited

eandre Jun 28, 2022

melkstam commented Aug 4, 2022

DomBlack commented Sep 29, 2022

		}
		// CallParameters is the type of the parameters to a method call, but require headers to be a Record type

		Support presets are:
		nextjs: Generates a TypScript client for use within a Next.js application

Support for Next.JS projects in generated clients #289

Support for Next.JS projects in generated clients #289

Conversation

DomBlack commented Jun 24, 2022 • edited

encore-cla bot commented Jun 24, 2022

eandre commented Jun 27, 2022

DomBlack commented Jun 27, 2022

DomBlack commented Jun 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eandre Jun 28, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melkstam commented Aug 4, 2022

DomBlack commented Sep 29, 2022

DomBlack commented Jun 24, 2022 •

edited

eandre Jun 28, 2022 •

edited