#Are you annoyed by the decorators? Do you wish the syntax was more implicit?

1 messages ยท Page 1 of 1 (latest)

dapper lava
#

Thoughts? Let us know.

remote stratus
#

I wouldn't say I'm bothered by them. I do wonder if they could be made redundant by for example exporting the class and privatizing methods not to be exposed, but they seem minimal enough. (I am more irked by the other "boilerplate" files -- package.json and tsconfig -- required by the TS SDK and might point to Deno as a way to eliminate that, but I do find the current level of boilerplate to be acceptable; minimal enough). Generally one of the reasons I prefer JS/TS is the lack of implicitness -- I dislike languages where symbols are just magically made available to the current context because of something like a namespace import; whereas in JS you can always track down the origin of any symbol directly from imports; otherwise it's a well-known platform global.

dapper lava
#

@molten gazelle

molten gazelle
# remote stratus I wouldn't say I'm bothered by them. I do wonder if they could be made redundant...

So in your opinion, package.json & tsconfig.json are not useful to generate? But what if you want to reconfigure an option of that runtime (in 90% case you shouldn't) but you might want to for some specific setup in your repo.
@empty bolt I wonder if I could remove the generation of these file and keep it internal to the runtime? But then what if you want to install an external npm package in your module? Do you think we could add some configuration to generate or not the files?

empty bolt
#

You could generate it only if the runtime doesn't support TypeScript without transpilation, like Bun, Deno, and soon Node too.

molten gazelle
#

That's right

remote stratus
# molten gazelle So in your opinion, `package.json` & `tsconfig.json` are not useful to generate?...

With the Node/Bun runtime you would still need those files for LSP support, so I get that they're still needed. Deno is the exception since it supports TS by default as well as imports by URL, and its internal LSP works fine with just a single file -- so even if you want to import an NPM package you don't have to have any extra files. Before Project Zenith was shipped I experimented with using Deno with the Dagger Client SDK since you could actually do everything in one Deno TS file -- liked this a lot since it didn't require committing any additional files. Now with Zenith/modules I don't know how LSP support for dagger modules would be incorporated to Deno's LSP, so maybe the point is now moot.

molten gazelle
hushed rapids
#

I'm not a fan of decorators in general, I find there are usually better ways to solve the problem they are used for

molten gazelle
# hushed rapids I'm not a fan of decorators in general, I find there are usually better ways to ...

Thanks for that feedback! So would you be against using decorators when declaring a Dagger object or property with the SDK? If so, do you have any suggestions on how we can manage the private/public properties of an object?

For example:

@object()
export class Example {
  @func()
  foo: string
}

Here, foo is a public property of Example and also can be query from a dagger call dagger call foo, but if we remove the decorators here, foo is just a simple public property of the object, not queryable from the dagger CLI.

If I change the logic to make all public properties callable from the CLI, how can you "hide" a property that you want to keep public in your object but hidden to the Dagger API?

I'm open to any suggestion ๐Ÿ˜„

Note: Maybe just adding a @hidden() decorator would do but I don't know if that's better than @func(), it's just the reverse ๐Ÿ˜ฆ

hushed rapids
#

I can see how using classes and decorators in typescript would seem to fit with other dagger SDK for OO based languages. Altough classes are part of javacript, for me they are not idiomatic. I would probably prefer using ESM modules as the encapsulation mechanism and export keyword to make functions/const/let vars public, and not use any classes at all. To me that is more idomatic javascript. However I can see how it does not fit with other languages. Javascript also does not support reflection like other langauges. I usually just have another well-known plain function that returns a data structure that represent what I would need for reflection purpose and have the framework call into that function to get the meta-data. This way everything can be solved with just plain functions. Of course this is all a matter of taste, using classes and decorators is another way to do the same thing and may be more in line with other languages.

#

One thing I find a bit annoying is excluding files using the decorators...
I currently have this

  @func()
  buildEnv(
    // Need to be careful so local files from ./node_modules/*/package.json etc. are not included
    @argument({
      ignore: [
        "**/*",
        "!apps/**/package.json",
        "!libs/**/package.json",
        "!package.json",
        "!pnpm-lock.yaml",
        "!pnpm-workspace.yaml",
        "!patches/",
        "!.npmrc",
      ],
    })
    source: Directory
  ): Container {
#

But I found that it does not really work if you call it from another function, so I have build that calls into build-env. For this I also need to repeat all the ignore files in the build function like this:

  @func()
  async build(
    @argument({
      ignore: [
        "**/*",
        "!apps/**/src/**",
        "!libs/**/src/**",
        "!apps/**/tsconfig.json",
        "!libs/**/tsconfig.json",
        "!apps/**/package.json",
        "!libs/**/package.json",
        "!package.json",
        "!pnpm-lock.yaml",
        "!pnpm-workspace.yaml",
        "!patches/",
        "!.npmrc",
      ],
    })
    source: Directory
  ): Promise<void> {
#

I tried to create a single constant to have the list of ignores and use that constant in the decorator @argument but I could not get that to work.
So in practice this is the only annoyance I have found yet. If I could declare a const of all ignore files and then create another const that reads from the first that would be easier to manage.
So what I tried was some thing like this:

const ignoreFilesBuildEnv = [
        "**/*",
        "!apps/**/package.json",
        "!libs/**/package.json",
        "!package.json",
        "!pnpm-lock.yaml",
        "!pnpm-workspace.yaml",
        "!patches/",
        "!.npmrc",
      ]
const ignoreFilesBuild= [
        ...ignoreFilesBuildEnv, "extra", "files", "not", "in", "the", "other", "list"
      ];

 @func()
  buildEnv(
    // Need to be careful so local files from ./node_modules/*/package.json etc. are not included
    @argument({ ignore: ignoreFilesBuildEnv })
    source: Directory
  ): Container {
  @func()
  async build(
    @argument({  ignore: ignoreFilesBuild 
})
    source: Directory
  ): Promise<void> {
#

Since the ignore lists are overlapping it would be nice to somehow not have to repeat them at two or more places.
Maybe I am just going about the ignore stuff the wrong way ๐Ÿ™‚

molten gazelle
# hushed rapids Since the ignore lists are overlapping it would be nice to somehow not have to r...

Hey, thanks for these feedbacks!

Why don't you try to have 1 constructor function to ignore most of the common file? So you can reduce a lot your deduplications?
Example:

@object()
export class Example {
  source: Directory

  constructor(
    @ignore([...])
    source: Directory
  ) {}

  @func()
  buildEnv() { .. const source = dag.Directory().withDirectory("/", this.source, { ignores: [<extra ignore pattern>] }
}

That might be a bit cleaner but I get the point, I agree that it would be simpler if I could handle decorators args as variable, but it's quite complex since I get the static value when introspecting the code.

hushed rapids
#

It would be possible to use variables if the mechanism was plain functions and data structures instead of class/decorators. Something like this:

export function build(source: Directory): Promise<Container> {
  // ...
}
export function build_meta(): MetaData {
  return { args: source: ignore: [...] }
}

In this example there is a convention that xxx function has a companion function xxx_meta that provides the meta data that is otherwise provided by decorators. This method fully avoids classes/decorators and allows the user to construct the meta-data with the full power of a regular function.

To be honest we do not use classes at all in typescript. We used to many years back when porting over from C# but there were so many pitfalls with this keyword and other stuff that we just started to fully avoid classes. Since then typescript has worked really great for us for many years now just using modules, types and functions. So I'm not really sure how to build stuff with classes in typescript ๐Ÿ™‚ In general my experience is that the typescript community does not favor classes unless trying to align something with C#/Java.

I don't know golang but looking at the dagger examples for golang they seem a lot closer to how I would prefer the typescript examples to look like as it seems they just use types and functions.

#

Regarding the constructor solution that seems interesting. For Dockerfiles I usually ignore everyting and then explicitly add what is needed. I'm trying to use the same approach for dagger. The reason is that we have a quite large monorepo with multiple apps. So there are just so many filles that not maximizing the ignoring increases the build time a lot. Is there some documentation about the approach in your example? I'm new to dagger so I'm not really sure how the constructor thing would interact with he CLI commands.