dynamic import/top level await breaks transpile

[xeaone]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure it has not already been reported

Version (i.e. v2.x.x)

1.0.1-alpha.109

Node.js version (i.e. 18.x, or N/A)

17

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10, or N/A)

11

Description

If we could replace the dynamic imports and to level awaits we would be able to transpile this to the users preferred es version. Because top level await and dynamic import is used we are way more limited with the es transpiled version. Any chance we could replace the top level awaits and the dynamic imports?

Steps to Reproduce

Change the build tsconfig target to es2015 and the module to umd

Expected Behavior

Be able to transpile to different es versions.

[fabiancook]

I do like the idea of being able to support backwards, but until this very point I hadn't ran into any issues with dynamic imports.. I really hope I don't need to avoid using it everywhere in the future just for tooling's sake.

I get it though.

I do have rollup running in this project which should be handling dynamic imports too.. but didn't look to far into it.

Swapping to an actual import just defeats the entire purpose of having dynamic imports available... which just makes me unsure on removing it.

The module is published as esm only, and all the other virtualstate modules are esm only, which may use dynamic imports around the place. Which makes me a bit more unsure.

In the future I will continue to use esm only in publishing, so surely these modules should be able to use dynamic imports.

Im just not fully convinced.

[fabiancook]

I'll definitely not be able to change the module from esm based to umd. The main package will be definitely type: module

[fabiancook]

I looked at the dynamic imports that are in use for this specific module.

All three are used to try and optionally polyfill other possibly already existing functions, but I have turned them into using a static import for now.

I would like to be able to use dynamic imports in all of these places, and this is the way of esm.... but I would rather people be able to at least try out the module for now.

For what its worth, I use the esm built version of this module across browsers, deno, and node, without any additional building tool past tsc

[fabiancook]

https://github.com/virtualstate/navigation/runs/7704088111?check_suite_focus=true#step:10:316

@virtualstate/navigation@1.0.1-alpha.110 has been published, if you could try it now as a dependency and see if the change did the trick.

[xeaone]

I am not suggesting to change this package from esm. I would think the goal or at least one goal of this repo would be to polyfill the navigation api. And usually you use a umd or iife for compatibility so that you can add it to the window. You could use an esm but that just limits the browsers that this module could help support. Maybe it is a setting I am missing but I tried using roll-up and I kept getting errors because of the dynamic imports. If roll-up can roll-up the dynamic imports I think that would eliminate the top level async and we could output an iife.

[fabiancook]

https://github.com/virtualstate/navigation/blob/89c77a9c387f77b7e45133e3c46dc442478adc95/scripts/post-build.js#L33

rollup is being used here, adding this should do the trick

[fabiancook]

The rolled up version though isn't currently exported (meaning I hopefully don't need to specifically support that umd build as well etc)

https://github.com/virtualstate/navigation/blob/89c77a9c387f77b7e45133e3c46dc442478adc95/src/tests/navigation.playwright.tsx#L47

It is only being used for testing in browsers where esm isn't/wasn't available.

[fabiancook]

... maybe just inlining the dynamic imports isn't what you're after though. I can see that rollup does process the dynamic imports, but it just includes them directly... maybe though its because I have ignored a bunch of files/modules?

[xeaone]

I will try that option to inline it!

[xeaone]

Okay so I did not realize that Skypack automatically transpiled and polyfilled. (Bind blown!)

Since browser support for es2017 has good coverage I think this initial issue is not relevant. But since the last update I am getting the following error code in the browser. And I think this is for version @virtualstate/navigation@1.0.1-alpha.111

Uncaught TypeError: Failed to resolve module specifier "abort-controller". Relative references must start with either "/", "./", or "../".

[fabiancook]

Would you be able to try out an import map, by adding this to your html file:

<script type="importmap">
{
  "imports": {
    "abort-controller":  "https://cdn.skypack.dev/abort-controller",
    "uuid":  "https://cdn.skypack.dev/uuid"
  }
}
</script>

Its not a long term solution, but I would have expected originally for AbortController to not be imported (as it is in global scope already), meaning this step would have never been needed.

[xeaone]

Yeah that seems to work.

[fabiancook]

Is it possible to try the older version of the module before the dynamic import changes. I think I might just put them back to the way it was.

[xeaone]

Certainly what version?

[xeaone]

So another reason to consider moving away from top level await is that this wont transpile with esbuild/esrm.run.

If you try to access the following cdn https://esm.sh/@virtualstate/navigation@1.0.1-alpha.111?target=es2017 you will get the following error:

/* esm.sh - error */ throw new Error("[esm.sh] " + "esbuild: Top-level await is not available in the configured target environment (\"es2017\")"); export default null;

Whats also weird is that skypack does not error when it transpiles and polyfills. For es2017 https://cdn.skypack.dev/@virtualstate/navigation/es2017. I guess they could be handling that some how but I am not to confident in that.

I might be able to spend sometime trying to adjust the package to remove the top level awaits.

[fabiancook]

I find it pretty unfortunate that this is where we are.

I need to use top await import to optionally pull in polyfills, or, always import them, for nothing, or use some other solution.

I just am not sure how to resolve this issue properly.

[fabiancook]

So another reason to consider moving away from top level await is that this wont transpile with esbuild/esrm.run.

It's not moving away, its non adoption, we can't adopt it.

I have a complete esm stack, every virtualstate & opennetwork repo should be esm, I can use top level await and dynamic imports in any of of these modules...

.... I guess its the pains of the "old" world coming through right

[fabiancook]

It seems all new runtimes have AbortController + crypto.randomUUID, these were the only things I were using top level await to get in this project. If top level await isn't easy to use for external users, I feel like throwing if a polyfill is needed will be.. somehow better

Before:

After:

Can see here an error will be thrown if not available:

https://github.com/virtualstate/navigation/blob/d104483f41dd9f1e335bb62c01631c69deba1191/src/import-abort-controller.ts#L1-L13

https://github.com/virtualstate/navigation/blob/d104483f41dd9f1e335bb62c01631c69deba1191/src/util/import-uuid.ts#L8-L10

[xeaone]

I will spend some time testing since your updates. If I have a possible solution can I submit a pull request?

[fabiancook]

For sure! Am definitely keen to hear if I can use top level await a bit more, I have used it in a bunch of modules, but will be a bit more mindful of it for now.

I think its pretty reasonable to push polyfilling out of the scope of this module if the globals are mostly available already.

https://caniuse.com/?search=crypto.randomUUID https://caniuse.com/?search=AbortController

node was the original runtime that didn't have AbortController... we're good now I think!

[fabiancook]

I noticed that there was still a path that used top level await, what happens with this in your bundler?

https://github.com/virtualstate/navigation/blob/9f51c89b622fa5a5b06c9b05fec8005541e23d74/src/navigation-entry.ts#L9

https://github.com/virtualstate/navigation/blob/9f51c89b622fa5a5b06c9b05fec8005541e23d74/src/util/uuid-or-random.ts#L1-L10

I am needing to add back an import to get node:crypto if there is no global available... hopefully this means that the import is never tried, and if it is, the catch is coming into play.

export const { v4 } = await import("./import-uuid")
  .catch(async () => {
    const crypto = await import("node:crypto");
    return {
        v4() {
            return crypto.randomUUID()
        }
    }
  })
  .catch(async () => import("uuid"))
  .catch(() => undefined)
  .then(
    (mod) =>
      mod ?? {
        v4(): string {
          return Array
              .from(
                  { length: 5 },
                  () => `${Math.random()}`
              )
              .join("-")
              .replace(".", "");
        },
      }
  );

If I don't do this, a real uuid isn't available in node, but is in bun, deno, & browser

[fabiancook]

Hi, I have added a polyfill thats rolled up and doesn't require loading of external dependencies. Does this solve your issues for your bundler? Or does the latest version work for you?

@virtualstate/navigation/polyfill/rollup is the import alias for the non esm polyfill. @virtualstate/navigation/rollup is the import alias for the esm rollup, which doesn't have any external dependencies (all included within where possible).
@virtualstate/navigation/polyfill is the esm compatible polyfill that will use async loading off dependencies where needed.

[xeaone]

Very exciting! I will give it a try.

[ar2r13]

@fabiancook looks like it doesnt help. I use esbuild as bundler it works only if add node:crypto to list of external modules

[fabiancook]

Damn. Newer versions of Node.js have a global crypto, but its not yet stable.... just like this module, so maybe I just swap over.

https://nodejs.org/api/globals.html#crypto_1

I can also maybe just put a "best effort" non standard uuid generator function in as a backup, right now its just a set of random numbers joined with -, kinda like a uuid but you would be able to spot immediately that its not a real uuid.... any suggestions welcome.

(Sorry too for the late reply, I have been on holiday for the last month and just returned :) )

---

This is just a side note though but doesn't help here, but is this an issue of the bundler rather than an issue of some code thats implemented as per JavaScript spec... I mean its code that runs across deno/node/bun/browsers... why not a bundler. Its like a bundler should be signing up to complete compatibility with valid code and provide work arounds itself if there is a problem like this. As a module developer I thought I did the exact thing that import was designed to do here, and have catch functions in there for imports that don't work. In the polyfill rollup, rollup ends up creating a similar issue, so I added some comments as some markers and replaced the code in that section with some compatible code...

See here for the work around I did for a polyfill bundle... https://github.com/virtualstate/navigation/blob/17132f9caabf98ee76fad459841e9c649346921b/example/polyfill-rollup.js#L309-L341

And this is the code that does the find and replace after build... its pretty hacky.

https://github.com/virtualstate/navigation/blob/17132f9caabf98ee76fad459841e9c649346921b/scripts/post-build.js#L215-L250

[fabiancook]

As per #16 (esbuild)

The default export includes a rollup/commonjs version of the module available.

https://github.com/virtualstate/navigation/blob/46ffcd8e8fa772f2ba7f48bf92a21eda9c6e2d45/package.json#L48-L51

Closing this issue with the assumption that it covers this use case. This was not in place when this issue was created.

Please re-open the issue if you continue to run into issues here.