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

[v1] Can't use top-level await in mesh.config.* #7342

Closed
4 tasks
beeequeue opened this issue Jul 23, 2024 · 13 comments · Fixed by #7360 or #7494
Closed
4 tasks

[v1] Can't use top-level await in mesh.config.* #7342

beeequeue opened this issue Jul 23, 2024 · 13 comments · Fixed by #7360 or #7494
Assignees

Comments

@beeequeue
Copy link
Contributor

beeequeue commented Jul 23, 2024

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

Make sure to fork this template and run yarn generate in the terminal.

Please make sure Mesh package versions under package.json matches yours.

  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

After switching to jiti for loading mesh.config.* it's no longer possible to use top-level awaits due to it converting the file to some format that doesn't allow it.

Not even .mjs allows it, presumably because it's being formatted to CJS.

To Reproduce Steps to reproduce the behavior:

Try to use a top-level await in a mesh config when it should be possible (ESM package)

Expected behavior

It should work

Environment:

  • OS: Mac
  • @graphql-mesh/...: v1 alpha
  • NodeJS: 20

Additional context

Jiti also has some other issues like bundling babel leading to our lambda bundle going from 1.7MB to 3MB after it was added.

@ardatan
Copy link
Owner

ardatan commented Jul 23, 2024

You should not use Serve CLI with lambdas.
For the environments that need bundling, @graphql-mesh/serve-runtime should be used.
You can see here;
https://the-guild.dev/graphql/mesh/v1/serve/deployment/serverless

@beeequeue
Copy link
Contributor Author

We are indeed using serve runtime for the lambda but are building the supergraph with the compose CLI.

Jiti is being included in serve-runtime as well due to being used in the utils package. I can send an example bundle metafile that you can inspect later

There are a bunch of packages that are required to be bundled despite not being used (eg Hive stuff) and I tried to see if I could improve that but it would require making createServeRuntime async so I gave up

But this issue is specifically about the transforming of the config file breaking using top level awaits when they should be possible

@ardatan
Copy link
Owner

ardatan commented Jul 23, 2024

Oh ok you're right. JITI should not be there already.

@beeequeue
Copy link
Contributor Author

@ardatan This issue was about top-level await not being usable in the config files, and it is still not possible

@enisdenjo
Copy link
Collaborator

hey @beeequeue, jiti is no longer used in utils which is what #7360 had done closing this issue. Furthermore, serve-runtime does not import any config files - that's done exclusively by the CLIs.

Maybe you're facing this issue while using compose-cli?

@beeequeue
Copy link
Contributor Author

beeequeue commented Aug 12, 2024

Yes, we're using a config file for composing our schema before deploying said schema with a lambda running serve-runtime.

However the old config from before jiti no longer works due to some required top-level awaits that should work.

The issue is about this, while the jiti-in-bundle was just another problem I found

@enisdenjo
Copy link
Collaborator

Gotcha! Is the compose config in TS or JS? Asking because a flag like --native-import that uses the native import function instead of jiti would work - as long as the config is JS.

@beeequeue
Copy link
Contributor Author

beeequeue commented Aug 12, 2024

The config is in TS, changing it to .mjs is trivial for us in this case.

Optimally jiti would just transform the file to the correct ESM/CJS format based on the package.json#type property and/or file extension. Right now .mjs gets transformed to CJS.

@enisdenjo
Copy link
Collaborator

Sadly because of unjs/jiti#72 (comment), I dont think jiti will support transformations to ESM any time soon.

I can have a --native-import flag PR-ed soon, if changing the config to JS is ok for you?

@beeequeue
Copy link
Contributor Author

It would work, yes.

@enisdenjo
Copy link
Collaborator

Ok cool, can you try the alpha compose-cli release and use the --native-import flag?

@graphql-mesh/compose-cli@0.8.0-alpha-20240812175659-32d432f1f0cb63fc5f50e7d739ee7a54e09a794e

@beeequeue
Copy link
Contributor Author

It works 🙂

@enisdenjo
Copy link
Collaborator

Great, thanks for helping out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment