#10680: json value

1 messages · Page 1 of 1 (latest)

steel rivet
#

🧵

#

@vestal lotus 👆 👋

#

10680: json value

#

I just pushed a temp commit that removs the old generated ts client

#

(I can't run pipelines on local changes, filesync upload is too slow on my starlink

steel rivet
#

more push and pray... trying to rename JSONValue to Value

#

any ideas @vestal lotus ?

#

I don't mind doing a hack manually, I just have no idea what to do

#

commenting out that tsc line, no idea what it does, don't care right now

#

doesn't help, I just get another error:

 .withExec(
  ┆ args: ["bun", "x", "rollup", "-c", "rollup.dts.config.mjs", "-o", "/out-node/core.d.ts"]
  ): Container! 0.5s ERROR

./dist/src/index.d.ts → ../out-node/core.d.ts...
[!] RollupError: Could not resolve entry module "./dist/src/index.d.ts".
    at getRollupError (/src/node_modules/rollup/dist/shared/parseAst.js:285:41)
    at Object.error (/src/node_modules/rollup/dist/shared/parseAst.js:281:42)
    at ModuleLoader.loadEntryModule (/src/node_modules/rollup/dist/shared/rollup.js:2269
    at async Promise.all (index 0)
! process "bun x rollup -c rollup.dts.config.mjs -o /out-node/core.d.ts" did not complete
  successfully: exit code: 1
#

rolling back

vestal lotus
#

It generates types for typescript generation

steel rivet
#

OK but from what inputs?

vestal lotus
#

The current TypeScript code

#

The generated typescript client looks good, I'm confused

#

Is it working fine with JSONValue?

steel rivet
steel rivet
#

Is what working fine with jsonvalue

#

@vestal lotus I changed the schema so that JSONValue.get(), JSONValue.set(), JSONValue.unset() etc no longer exist

#

Normally the SDKs should just re-generate everything from the new schema

#

But they're not - there is a trace of the old schema left somewhere in the codegen pipeline, which bricked everything

#

I guess I'll have to manually check out sdk/typescript/src/api/client.gen.ts from main, in order to re-generate

#

basically you have a circular dependency in your typescript codegen pipeline @vestal lotus

#

if client.gen.ts breaks, you can't fix it anymore without manual intervention

#

testing my theory...

#

the circular dependency is:

sdk codegen -> engine build -> bundle sdks in engine -> sdk codegen

#

not sure which of those is wrong

vestal lotus
vestal lotus
steel rivet
steel rivet
vestal lotus
#

I don't understand why it struggles to generate your files

steel rivet
#

Yeah I know what the error is 🙂

#

I'm wondering if it's maybe because an earlier codegen step crashes the engine? I've been getting engine crashes a few times while developing

#

So maybe that's why you never saw it before?

vestal lotus
#

I don't know but if you do dagger -m .dagger call sdk typescript generate -o ., what happened?

steel rivet
#

Another theory is that it's specifically JSONValue that confuses the codegen because of weird capitalization

#

I renamed to Value for now. So far, no issue

vestal lotus
#

That' s possible, this will be fixed once we move to a custom json schema, there will be no naming issue anymore (/cc @rapid pebble)

steel rivet
#

Would love to learn more

vestal lotus
#

Basically right now, all SDKs gets a GraphQL schema and then each of them implement some logic to resolve type, handle case conversion, naming, see if a function is a resolver or chainable etc, all of that is handle internally by each generator to generate the right output.

Instead we will abstract that to the API so the SDK will get a custom JSON schema instead of that GraphQL schema where all that complexity is abstract, so generators will only generate or not implement any logic to resolve things in the shema

#

So for example, JSONValue would be exposed with different fields based on the case, like json_value, JSONValue, JsonValue etc

#

and the sdk can pick up its case

#

That's an example, may change with implementation

steel rivet
#

@vestal lotus it seems like the SDK codegen has a major issue with a type named JSONValue... I have no choice but to rename it to JsonValue for everyone

#

Yup @vestal lotus it's a bug in the codegen

#

The same generated client mixes 2 different capitalizations

#

@rapid pebble @valid sphinx do you know how the "capitalizer" part of the codegen might make this sort of mistake? Looks like a special case for the word JSON maybe?

#

Is there a hardcoded list of special-case words maybe? And then it's not actually applied 100% consistently?

#

(this is for the core SDK codegen, ie. when developing a new feature in the engine API)

rapid pebble
#

Yeah, it's not applied 100% consistently. We're using two libraries in different places still, I think.

#

JSONValue would be a Go convention in terms of casing. GraphQL follows JavaScript and while there's no strict rules there, it's more common to use JsonValue, i.e., without capitalizing acronyms.

steel rivet
#

But we have JSON already

#

The issue is not with my choice of GraphQL type, but in how the typescript codegen translates it to JS --> I don't care what it generates but obviously it should stick to generating only one, to avoid this crash

#

Every other SDK is handling JSONValue just fine

rapid pebble
#

Which codegen are you having this issue? Is it the TypeScript SDK?

steel rivet
#

Yes, core sdk

rapid pebble
#

By "core" sdk do you mean the Go SDK? I'm confused.

steel rivet
#

You can repro with dagger -i -m github.com/shykes/dagger@jsonvalue -c 'dev'

steel rivet
rapid pebble
#

So you mean the client bindings codegen for TypeScript SDK?

steel rivet
#

yes

rapid pebble
#

But in TS the convention would be JsonValue, not JSONValue

steel rivet
#

but since we also use "TypeScript SDK" for the codegen in modules, I was disambiguating

steel rivet
#

I just want it to not crash the build 🙂

rapid pebble
#

I usually refer to "client bindings codegen" vs "module codegen".

steel rivet
#

Note that the codegen itself doesn't fail. It outputs an incorrect client with inconsistent types (the one I copied into the gist), which then causes engine build to fail

rapid pebble
#

Let me look

steel rivet
#

That's why my repro command above is just dev

rapid pebble
#

I suppose maybe TS keeps the casing from GraphQL, right @vestal lotus? Rather than convert it.

#

Since they're the "same" ecosystem.

steel rivet
vestal lotus
steel rivet
#

You can see that the previous commit (graphql type=JsonValue) doesn't have the issue:

dagger -i -m github.com/shykes/dagger@0b5995f343b6644ad00c2dd88480feb9339f3b20 -c 'dev'

rapid pebble
#

If it's called JsonValue in the API, Go should codegen it to JSONValue, right?

steel rivet
#
  • JSONValue❌dagger -i -m github.com/shykes/dagger@472208c180f75bd8b530067e9586d256fc9437cf -c 'dev'
  • JsonValue✅ dagger -i -m github.com/shykes/dagger@0b5995f343b6644ad00c2dd88480feb9339f3b20 -c 'dev'
steel rivet
rapid pebble
#

Go's convention to capitalize acronyms is a major PITA. It gives all sorts of issues trying to keep it all consistent.

steel rivet
#

Complicating my situation, my devloop is very slow and brittle right now: parc + slow internet = infinitely long filesync, so I have to push-and-pray everything, and juggle the brittle sequence of commits in my head every time I try anything

vestal lotus
#

Why using parc if you internet is slow? Can't you run things locally?

steel rivet
#

My internet isn't actually that slow, it's like a regular DSL line

#

It's just PARC+filesync that somehow hates it

#

Anyway yeah, I might have to switch my setup back to local.

vestal lotus
#

Ohhh weird, most of it should be cached after the 1st run

rapid pebble
#

You may be uploading more context than you intend.

vestal lotus
steel rivet
rapid pebble
steel rivet
#

My life right now

success.png
#

mmm maybe it's a regression related to blueprints? It does mess with context directory access

rapid pebble
#

When I run your command what I see is this. Are you getting a more specific error?

Screenshot_2025-07-23_at_12.24.31.png
steel rivet
#

nope, that's the error

#

oh sorry - yes

#

that's a TUI error hoisting problem

#

(add that to the stack of problems...)

#

if you re-run the command in that debug shell, you should see a very clear tsc error

rapid pebble
#

I see it now

steel rivet
#

@rapid pebble it's related to those 2 gist links I shared above

#

(mismatched capitalization in the same file)

rapid pebble
#

@vestal lotus, is it that when it's the type it's printed as is, but then for the options you generate the new name entirely according to TS conventions instead of just concatenating the field name and "Opts" to the type name?

#

So since we do have JSON, I suppose it doesn't have an Opts object so this hasn't been triggered before.

vestal lotus
rapid pebble
#

It's converting parentName to PascalCase: {{ $parentName | PascalCase }}

#

Should probably keep $parentName as is.

vestal lotus
#

Ok, will work on a fix today

steel rivet
#

Thank you @vestal lotus !

rapid pebble
#

@steel rivet, maybe you can try in your branch to see if it solves everything for you? Just remove that | PascalCase filter.

steel rivet
rapid pebble
steel rivet
#

@vestal lotus I'm returning to this PR today, should I still do a manual fix?

vestal lotus
#

I didn't had time yesterday, I was stuck on that git ignore PR

#

However, I checked and yeah there's a deadlock in the TS codegen when running the dev engine

vestal lotus
#

I'm on it right now

#

My other PRs are green

steel rivet
#

Ah now I see the fix you suggested is not to the generated file, but to the codegen itself... facepalm

#

hum, I tried removing the | PascalCase but it didn't fix the issue

# 
diff --git a/cmd/codegen/generator/typescript/templates/src/method.ts.gtpl b/cmd/codegen/generator/typescript/templates/src/method.ts.gtpl
index 5140b4b79..5c74108fa 100644
--- a/cmd/codegen/generator/typescript/templates/src/method.ts.gtpl
+++ b/cmd/codegen/generator/typescript/templates/src/method.ts.gtpl
@@ -23,8 +23,8 @@
     {{- if $optionals }}
         {{- /* Insert a comma if there was previous required arguments. */ -}}
         {{- if $required }}, {{ end }}
-        {{- "" }}opts?: {{ $parentName | PascalCase }}{{ .Name | PascalCase }}Opts {{- with .Directives.SourceMap }} // {{ .Module }} ({{ .Filelink | ModuleRelPath }}) 
-        {{ "" }} 
+        {{- "" }}opts?: {{ $parentName }}{{ .Name | PascalCase }}Opts {{- with .Directives.SourceMap }} // {{ .Module }} ({{ .Filelink | ModuleRelPath }})
+        {{ "" }}
         {{- end }}
     {{- end }}
vestal lotus
#

In method_solve 😛

#

The PR will be open soon, 1 sec I'm writing my commit message

#

I added a test too to verify the behaviour

steel rivet
#

Nice thanks

#

Quick question @vestal lotus, what's your setup for working on multiple PRs in parallel? I don't use git worktrees so I'm pretty slow to context switch... These days I have 3-4 pans on the fire so it slows me down

#

do you have a custom git worktree layout?

vestal lotus
#

No specific setup, I just switch PR when my CI is green so I'm sure I can go on another while I wait for reviews

steel rivet
#

@vestal lotus rebasing on your PR & testing

#

oh wait @vestal lotus ... I can't just do dagger -c 'generate | export .' after applying your fix

#

I have to run that with dagger-in-dagger correct?

vestal lotus
#

dagger -m .dagger call sdk typescript generate -o .

steel rivet
#

yeah same thing. I mean I have to run that with the dev dagger

vestal lotus
#

yeah

steel rivet
#

crazy that we can just do that, and it works

vestal lotus
#

What do you mean?

steel rivet
#

running a patched dagger codegen inside a bootstrapped build of dagger, turtles all the way down, and it's reliable

vestal lotus
#

Ohhh yeah haha

steel rivet
#

99% of real world software projects don't have that without a LOT of custom scripts, and it's a nightmare

vestal lotus
#

Dagger saves the day haha

steel rivet
#

Here's what I ran:

dagger <<.
 dev |
 with-directory . . |
 with-exec -- dagger -c 'sdk | typescript | generate | export .' |
 directory . |
 export .
.
vestal lotus
#

Why just not dagger -m .dagger call sdk typescript generate -o .? Much less verbose for the same result

steel rivet
#

final diff:

diff --git a/sdk/typescript/src/api/client.gen.ts b/sdk/typescript/src/api/client.gen.ts
index aba2b97ac..07d11e5e3 100644
--- a/sdk/typescript/src/api/client.gen.ts
+++ b/sdk/typescript/src/api/client.gen.ts
@@ -6772,7 +6772,7 @@ export class JSONValue extends BaseClient {
    * @param opts.pretty Pretty-print
    * @param opts.indent Indent each line by the given number of whitespaces
    */
-  bytes = async (opts?: JsonvalueBytesOpts): Promise<JSON> => {
+  bytes = async (opts?: JSONValueBytesOpts): Promise<JSON> => {
     if (this._bytes) {
       return this._bytes
     }
vestal lotus
#

Yeah, it should have fixed your problem normally

steel rivet
vestal lotus
#

I always do that to regenerate my code and it works fine

steel rivet
#

-m .dagger does nothing as far as I know

#

it's -m <random_subdir>, it just finds the nearest dagger module which is .

vestal lotus
#

I mean inside the command

#

Try, revert the change and just run the command I sent, you should get the same result

#

Your shell command is doing dagger in dagger in dagger

steel rivet
# vestal lotus yeah

OK so here the answer was "no you don't need to, because our codegen pipeline already sets up a dagger-in-dagger"

#

btw -m .dagger still does nothing I think

# 
dagger -c 'sdk | typescript | generate | export .'
vestal lotus
#

No it does not, but what I meant by -m .dagger is that the module your calling already sets up a dev engine

vestal lotus
steel rivet
#

I always use shell syntax, so I don't have to juggle between 2 syntaxes...

#

dagger call with no arguments is great, then you have to add one argument and it's a PITA

#

OK almost there! thanks for the fix

vestal lotus
steel rivet
#

@vestal lotus can you squash please?

#

@vestal lotus I'll take care of it

vestal lotus
#

I squash when I merge my PR

#

Just approve it, I'll merge it

steel rivet
#

I hate the ugly concatenated commit message from that button

#

I'm doing it the docker way 🙂

vestal lotus
#

No but I remove everything and rewrite a clean message each time haha

steel rivet
#

I didn't know that was possible from the web ui

vestal lotus
#

Ah I didn't know that

#

I think everyone were doing it haha

steel rivet
#

OK go ahead and merge it then

#

I've been stuck on this PR for a week, trying to move on

vestal lotus
#

Like that

Screenshot_2025-07-24_at_14.18.17.png
steel rivet