winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.94k stars 194 forks source link

Top-level await in extern JS files does not work #7085

Open Chriscbr opened 1 week ago

Chriscbr commented 1 week ago

I tried this:

I wrote this program using some extern JS code:

// main.w
bring cloud;

class Util {
  pub extern "./util.js" static inflight double(value: num): num;
}

test "foo" {
  log(Util.hash("hello"));
}

// util.js
await new Promise((resolve) => setTimeout(resolve, 1000));
console.log("finished async work");

export const double = async (value) => {
    return value * 2;
}

This happened:

✘ [ERROR] Top-level await is currently not supported with the "cjs" output format

    util.js:3:0:
      3 │ await new Promise((resolve) => setTimeout(resolve, 1000));
        ╵ ~~~~~

✘ [ERROR] This require call is not allowed because the imported file "util.js" contains a top-level await

    target/test/main.wsim/.wing/inflight.Util-1.cjs:7:22:
      7 │       return (require("../../../../util.js")["hash"])(query)
        ╵                       ~~~~~~~~~~~~~~~~~~~~~

  The top-level await in "util.js" is here:

    util.js:3:0:
      3 │ await new Promise((resolve) => setTimeout(resolve, 1000));
        ╵ ~~~~~

node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);

I expected this:

No error

Is there a workaround?

Avoid top-level await, instead adapting the code to something like:

// util.js
const work = new Promise((resolve) => setTimeout(resolve, 1000));
work.then(() => console.log("finished async work"));

export const double = async (value) => {
    await work;
    return value * 2;
}

Anything else?

No response

Wing Version

0.84.1

Node.js Version

20.12.2

Platform(s)

MacOS

Community Notes

Chriscbr commented 1 week ago

Maybe we can change the default bundling format to ESM?

eladb commented 1 week ago

I am not sure what the implications are but I am generally in favor. @MarkMcCulloh if you are around, would love your opinion

MarkMcCulloh commented 1 week ago

Yep, it makes sense to try bundling ESM. The changes required will likely go passed bundling though: wingc will need to emit await import() instead of a require() for inflight extern imports because esbuild will get mad otherwise. Doing this will likely necessitate other changes in the SDK to avoid the exports syntax. Some CJS stuff like require.resolve will likely need to be shimmed in as well.

Keeping this limited to inflight for now reduces the blast area of necessary changes because it gets invoked in a fresh node process.

eladb commented 1 week ago

Thanks man

On Mon, Sep 9, 2024 at 4:18 PM Mark McCulloh @.***> wrote:

Yep, it makes sense to try bundling ESM. The changes required will likely go passed bundling though: wingc will need to emit await import() instead of a require() for inflight extern imports because esbuild will get mad otherwise. Doing this will likely necessitate other changes in the SDK to avoid the exports syntax. Some CJS stuff like require.resolve will likely need to be shimmed in as well.

Keeping this limited to inflight for now reduces the blast area of necessary changes because it gets invoked in a fresh node process.

— Reply to this email directly, view it on GitHub https://github.com/winglang/wing/issues/7085#issuecomment-2338104671, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAESGDGSDKIX4NOAXHF2KCDZVWN2HAVCNFSM6AAAAABNZCJ4QKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZYGEYDINRXGE . You are receiving this because you commented.Message ID: @.***>