Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable language support for adaptors when writing job code #9

Open
josephjclark opened this issue Jan 6, 2025 · 13 comments
Open

Enable language support for adaptors when writing job code #9

josephjclark opened this issue Jan 6, 2025 · 13 comments
Assignees

Comments

@josephjclark
Copy link
Collaborator

We need to download the adaptor API and present it to the typescript context.

Maybe we can re-use some trick used by Lightning and monaco?

@doc-han doc-han self-assigned this Jan 15, 2025
@doc-han
Copy link
Collaborator

doc-han commented Jan 20, 2025

@josephjclark
Is ast.json a good place to get adaptor functions info for language support?

It seems I can get ast.json for adaptors by version via github.
https://github.com/OpenFn/adaptors/blob/@openfn/language-common@<version>/packages/common/ast.json

eg.
https://github.com/OpenFn/adaptors/blob/@openfn/[email protected]/packages/common/ast.json
https://github.com/OpenFn/adaptors/blob/@openfn/[email protected]/packages/common/ast.json

@josephjclark
Copy link
Collaborator Author

Hey @doc-han , ast.json is OK I guess but I don't understand why you need it? The .d.ts files should be the best source of truth for VSC.

If you're having to manually generate some kind of data structure to drive VSC, well, we need to talk about that! It doesn't feel like the right approach.

Can you book some time in my calendar to talk about this tomorrow?

@josephjclark
Copy link
Collaborator Author

Updates from our call:

Basically we want to pretend that we have a regular javascript (or typescript?) project here, and we want to configure it so that each job file is properly scoped. Maybe with globals, maybe with imports.

As a reference for how we get language support today, see https://github.com/OpenFn/adaptors/wiki/How-to-get-code-assist-for-adaptors-in-VSC

  • Don't worry about .fn file extensions. If they're blocking anything, drop them
  • I am quite happy to pretend that job files are TypeScript if it helps
  • The ask here is to somehow inject type definitions from d.ts files into each job file, using as "normal" javascripty stuff as possible
    • Ideally there is no kind of explicit declaration at the top of the file. Maybe we can hide it? Maybe we can set a mapping somewhere?
    • I'd happy accept a ///<reference comment to inject the dts files. Maybe we can autogenerate/auto-insert it from the workflow?
    • I'd also accept an explicit import { ... } statement. Again, best if we can auto-generate it!

If we really, really can't get this working, I'm happy explore using ast.json or describe-package and building our own code complete API. It's very impressive to see you've got that working by the way. I'm just hoping for a cheaper, more "normal" win here!

@doc-han
Copy link
Collaborator

doc-han commented Jan 22, 2025

///<reference

  1. ⛔️ It doesn't respect compilerOptions.paths. Hence, you need to provide an absolute path for every inclusion.
  2. ⛔️ It just doesn't work. It doesn't make types available in a file but rather instructs the compiler to consider some additional types for the file during compilation.

import { ... }

  1. ✅ This works given compilerOptions.paths points to the external repo location. User can import and use modules
  2. ⚠️ The imports will have the adaptor version attached to it. Hence, we need a compile step to clean up all imports before doing actual compiler work.
    Eg.
import { get } from '@openfn/language-http_4.3.3'
  1. ⛔️ We can't hide the imports.
  2. ⚠️ We can auto-generate something like import {} from @openfn/language-http_4.3.3 then the users will start expanding the empty object with what they have used(on a newly created file with no content that just got connected via workflow.json). How about already existing codebases? I'm not sure we want to parse the file to know what functions they've used to do the imports for them. When a users is using v4.3.3 and then changes version in workflow.json. We then have to change this import definition to match the new defined version.

Note: Code written in lightening will end up being different from those in vscode.

  1. Directly exported code from lightning won't work out of the box. We'll have to add import & find used functions and populate the empty import declaration.
  2. Code from vscode going to lightning will need import cleanup.

cc: @josephjclark

@josephjclark
Copy link
Collaborator Author

This is great analysis @doc-han, thank you!

Let me think about it. I agree that this just isn't going to fly in its current state. We're so close and yet so far...

@josephjclark
Copy link
Collaborator Author

@doc-han Thinking about this overnight. What do the other JS runtimes do? Bun and Deno, for example, must both have vscode extensions with language support. And I know for a fact that Bun introduces different things to the global environment, I'm sure deno does too.

Can you have a look for a bun extension and see what they do? Have they found a way to register extensions, or customise the scope, or do they create their own language support from scratch?

@doc-han
Copy link
Collaborator

doc-han commented Jan 23, 2025

Can you have a look for a bun extension and see what they do? Have they found a way to register extensions, or customise the scope, or do they create their own language support from scratch?

For type support. They declare one global namespace Bun.
And then provide their types to tsconfig.
hence, you can do Bun.serve without any imports in the file.

@josephjclark
Copy link
Collaborator Author

Ok, and that works because it's global. It's the same environment for all bun JS.

Our problem is more like: each file has its own vm/environment

@josephjclark
Copy link
Collaborator Author

Ok this is probably very over elaborate, but what if:

  • We put each step.js file in its own folder
  • We generate a tscofnig in the same folder for that step

Would that apply the right types?

@doc-han
Copy link
Collaborator

doc-han commented Jan 23, 2025

Ok this is probably very over elaborate, but what if:

  • We put each step.js file in its own folder
  • We generate a tscofnig in the same folder for that step

Would that apply the right types?

  1. Even if that works, the real bottleneck is that we'll have to define all adaptor functions as global declarations for the types to be available in the file.
  2. Looks like our vscode extension which was supposed to make work easier is rather multiplying it. a directory per job.

@josephjclark
Copy link
Collaborator Author

@doc-han Well the idea is to push the problem further up the hill until we run out of options, and then see what the least-bad implementation looks like. But I hear you.

I'm sure we should be able to achieve this 🤔 But I'm not willing to invest too much more time in it. Let's put a pause on it until next week.

One final question: how long do you need to finish a usable demo your own custom language support? Just enough to be useful?

@doc-han
Copy link
Collaborator

doc-han commented Jan 24, 2025

Yeah make sense.
After several testing around isolating the jobs. I think the best is to make use of eslint to restrict the imports. https://eslint.org/docs/latest/rules/no-restricted-imports

Hence, in a job directory.

  1. we have a scoped eslint config that makes sure that the files in there can only import a specific adaptor.
  2. a tsconfig that has paths mapping an adaptor import to it's actual expected version.

inbox: a zip of a working openfn project with this. You can change versions in tsconfig to match what you have installed at /tmp/openfn/repo or install [email protected] & [email protected]

@doc-han
Copy link
Collaborator

doc-han commented Jan 24, 2025

With this we will have imports using @openfn-common or what we want and it will be mirrored to the actual version. Then we don't have any issue with compiler cleanups as mentioned earlier on because the version wouldn't be in the import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants