base JS/TS class prepared for method hook ups #58

jbolda · 2025-08-20T17:36:10Z

Motivation

When you are generating APIs, especially from another language, it tends to produce code that isn't idiomatic or otherwise doesn't feel like it was written in the target language. To solve for this, we added a bit of scaffolding and some tests to show usage intent. This allows us to specifically craft the public API that we want, and then the internal methods and APIs can be more freely adjusted at the need of technical requirements without affecting the public API.

Approach And Context

For additional context, I think we want to look to shift our Parser in the Rust wasm-bindgen setup into more strictly functions. From what we have looked at, I think it will be easier to keep the TypeScript generation up to date and current if it is more focused.

pkg-pr-new · 2025-08-20T17:38:33Z

cooklang-playground

npm i https://pkg.pr.new/cooklang/cooklang-rs/@cooklang/cooklang-ts@58

commit: ff45a89

michalmoc · 2025-08-21T09:50:00Z

typescript/src/parser.ts

+    return this.rawContent;
+  }
+
+  #handleFunctionalOrInstance(instanceInput: string | undefined) {


Looks like 2 classes to me, perhaps inheriting some common code?

I agree with @michalmoc here. I was worried about a user reaching the error conditions within this method, when they can be avoided with an improved design.

We might want to take inspiration from yaml, Markdown, and Date.

Here's an example:

import { CooklangRecipe, CooklangRenderer } from "@cooklang/parser"; const recipeBlank = new CooklangRecipe(); const recipe = new CooklangRecipe("Write your @recipe here!", {someOption: true}); const recipeClone = new CooklangRecipe(recipe); console.log(recipe.toString()); // Same as render(CooklangRenderer.BasicString) console.log(recipe.render(CooklangRenderer.PrettyString)); console.log(recipe.render(CooklangRenderer.Html)); // TODO sample response // Since CooklangRecipe initialization already did the parse, to access data simply: console.log(recipe.metadata); // TODO sample response console.log(recipe.ingredients); // TODO sample response console.log(recipe.sections); // TODO sample response

Correct. There are two classes @michalmoc. One extends the other though so it does inherit that code.

@panzer Aye, I understand the concern for these errors. An alternative that I had considered is having two completely separate paths whereby if you give a value to the initial constructor, it will use a class specifically intended for "instance use" or the "functional" use. I do like the though of adding a toString() which could mirror the "pretty string" perhaps.

There is a bit of a balance to be had, as we don't want to require building up so many classes / objects, as a user may be generating or rendering 1000s of these. Part of the reason I went through this exercise as exactly this discussion. We want to be able to discuss the public API, and it is much easier to share input after doing this. Happy to discuss and work through it.

By two classes I mean: if you have the same if in every major method, then obviously those are two different implementations, i.e. two classes deriving from the same one.

Ah, I see. We do have effectively two separate code paths with lots of overlapping functions. We could make the classes smaller and do more extends to separate out the code paths and remove #handleFunctionalOrInstance from each. As I was talking through it and organically built it up, I pulled out #handleFunctionalOrInstance as a separate method. In retrospect, I did consider splitting the code paths more if only to make the types and uses of more obvious in your IDE.

michalmoc · 2025-08-21T09:52:29Z

typescript/src/parser.ts

+  }
+
+  // TODO create issue for this
+  renderPrettyString(recipeString?: string) {


We will need more render types, e.g. renderObsidian which will be similar to renderHtml but not identical. For that reason I'd propose to use visitor pattern, similar to rendering calendar in python - render function would walk through structure and visitor will handle actual rendering allowing easy reuse of code via inheritance.

Yeah, I'd prefer if additional processing on top of the parsed-data would be separate from this class

@michalmoc Hmm, yea I haven't looked at the Obsidian or VSCode implementation. I think to @panzer's point, we probably don't want to overload this class. I presumed that in the cases where we need to walk the tree, we would rely on parseRaw. Have you looked at the existing implementations? Would that be sufficient?

@panzer are you suggesting that we actually remove the renderHTML and renderPrettyString from this class?

Yeah that is what I'm suggesting. I thinking building in certain renderers into the core API should be done cautiously. Simple toString render is IMO necessary, but HTML, markdown, png, audio-file... we open up the flood gates potentially.

Aye, in my mind, this was the limit of what we should do in the core. Sounds like HTML is the only one in question? I am not hard set on it being part of core, but I think having it minimally available somewhere in this package is useful. Helpful for those plan VanillaJS type demos / examples.

That makes sense. I can get onboard with HTML as one that we ship with the package. I did want to make sure we had a plan for other types of conversion/rendering which seems to be a common request. Obsidian being one example. What you've put forward makes sense!

michalmoc · 2025-08-21T09:59:57Z

typescript/src/parser.ts

+  }
+}
+
+class CooklangParser extends CooklangRecipe {


Parser being its result feels weird. Is it some JS idiom? If not, woudn't it be better if parse() just returned CooklangRecipe?

Sorry, but I don't understand your question. With the functional usage, it does.

I mean you basically change the type, from Parser to CooklangRecipe, and this change is hidden from type system and therefore from the user. It feels to me that normal solution would be "parser returns recipe", not "parser becomes recipe".

We also control Parser in this package. That Rust file is only for this package. I anticipate that Rust file will change anyways if only to simply it to make type generation more robust.

michalmoc · 2025-08-21T10:04:23Z

typescript/src/parser.ts

+    if (this.rawContent) {
+      this.setRecipe(parsed);
+    }
+    if (!this.rawContent && recipeString) {


guaranteed by #handleFunctionalOrInstance(recipeString), no need to check again

Yea.... we know this, but Typescript was struggling to properly infer this. It kept expecting the return to possibly be undefined. If we can figure out a better way to type narrow, happy to adjust this.

michalmoc · 2025-08-21T10:10:42Z

typescript/src/parser.ts

+  sections = [];
+  cookware = new Map();
+  timers = [];
+  constructor(rawParsed?: ScaledRecipeWithReport) {


Copying interface from Number, perhaps we want a function parseRecipe and make this constructor take raw string and call it.

This class isn't exported. With that we can always call the parse beforehand. Also this thought may be related here.

panzer · 2025-08-20T23:51:36Z

typescript/src/parser.ts

+const Parser = RustParser;
+export { version, Parser, type ScaledRecipeWithReport };
+
+class CooklangRecipe {


I'm sure you have a great reason for this, but I was a bit confused why we wouldn't use the interface Recipe that is exposed by the wasm build. It neatly provides types for each of the members as well.

On one hand, we do expose the TS package to a wide array of changes on the Rust package. On the other hand, we might have a lot of manual work to keep the packages in-sync with each other.

What do you think?

This isn't purely for types. My expectation which in retrospect didn't come across very obvious is that we use the types generated from the WASM build in this class. Some we may be able to use directly, like ingredients. However, the sections is rather verbose. It felt reasonable to provide a bit of sugar on top to improve the ability to render it.

I split this off as it's own class so we could access each of these like a property on an object, e.g. recipe.ingredients.

panzer · 2025-08-21T12:51:01Z

typescript/src/parser.ts

+  }
+
+  // TODO create issue for this
+  renderPrettyString(recipeString?: string) {


Yeah, I'd prefer if additional processing on top of the parsed-data would be separate from this class

panzer · 2025-08-21T13:20:18Z

typescript/src/parser.ts

+    return this.rawContent;
+  }
+
+  #handleFunctionalOrInstance(instanceInput: string | undefined) {


I agree with @michalmoc here. I was worried about a user reaching the error conditions within this method, when they can be avoided with an improved design.

We might want to take inspiration from yaml, Markdown, and Date.

Here's an example:

import { CooklangRecipe, CooklangRenderer } from "@cooklang/parser"; const recipeBlank = new CooklangRecipe(); const recipe = new CooklangRecipe("Write your @recipe here!", {someOption: true}); const recipeClone = new CooklangRecipe(recipe); console.log(recipe.toString()); // Same as render(CooklangRenderer.BasicString) console.log(recipe.render(CooklangRenderer.PrettyString)); console.log(recipe.render(CooklangRenderer.Html)); // TODO sample response // Since CooklangRecipe initialization already did the parse, to access data simply: console.log(recipe.metadata); // TODO sample response console.log(recipe.ingredients); // TODO sample response console.log(recipe.sections); // TODO sample response

panzer · 2025-08-24T01:14:55Z

typescript/test/parser.bench.ts

+  // technically saves a few cycles on the actual
+  // bench result vs the instance bench, but this is
+  // effectively the use case where you init once
+  // and reuse that parser over and over


Good to point out that this is a major optimization! I'm thinking the "right way" should be the way we guide users towards through the library design. That's why I used the static parser in #60.

To say it another way: I don't see the benefit of "instance" parsing. What do you think?

base JS/TS class prepared for method hook ups

754a17a

jbolda requested review from dubadub and a team August 20, 2025 17:36

michalmoc reviewed Aug 21, 2025

View reviewed changes

panzer reviewed Aug 21, 2025

View reviewed changes

panzer mentioned this pull request Aug 22, 2025

Typescript API spec proposal #60

Open

jbolda added 2 commits August 22, 2025 01:31

add simple bench, wire up parse for better bench

b071f3f

change .raw reparses

ff45a89

panzer reviewed Aug 24, 2025

View reviewed changes

jbolda added 2 commits September 14, 2025 01:20

refactor in hopefully more clear use cases

9a4c99e

split into separate files

6a32ee4

base JS/TS class prepared for method hook ups #58

Are you sure you want to change the base?

base JS/TS class prepared for method hook ups #58

Uh oh!

Conversation

jbolda commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Approach And Context

Uh oh!

pkg-pr-new bot commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panzer Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panzer Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jbolda commented Aug 20, 2025 •

edited

Loading

pkg-pr-new bot commented Aug 20, 2025 •

edited

Loading

panzer Aug 21, 2025 •

edited

Loading

panzer Aug 21, 2025 •

edited

Loading