AngleSharp / AngleSharp.Js

:angel: Extends AngleSharp with a .NET-based JavaScript engine.
https://anglesharp.github.io
MIT License
103 stars 22 forks source link

Added support for JS modules and importmaps #93

Closed tomvanenckevort closed 6 months ago

tomvanenckevort commented 6 months ago

Types of Changes

Prerequisites

Please make sure you can check the following two boxes:

Contribution Type

What types of changes does your code introduce? Put an x in all the boxes that apply:

Description

As described in #92, this PR adds support for handling new ECMA JS modules and importmaps.

Note: to enable the deserializing of the importmap JSON I added a reference to System.Text.Json to the project, which would become an additional dependency of the NuGet package. I don't see any issues with that, but let me know if you think otherwise.

tomvanenckevort commented 6 months ago

Also the scopes for importmodules are imho not correctly in there - but I think this will be difficult if Jint does not support dynamic resolution.

Could you elaborate a bit more on that?

FlorianRappl commented 6 months ago

Could you elaborate a bit more on that?

Yeah so what I meant is that the scope field gives ESM modules different parts, e.g.:

Now starting with some imports each one would resolve "react" to "react1.mjs", while an import of a module "foo/something.mjs" would find "react2.mjs" when importing "react". This, however, cannot statically be set, but only when resolving the module (i.e., when Jint would find an "import" statement it would go to the dynamic resolver with the current module's metadata - most importantly its URL - to find what module is actually meant / requested here).

The scope's are not dependent on the document's path (surely, in a way they are - as this is the base path for all relative paths, but once you import from https://esm.sh/... other modules are also imported relative to this one).

tomvanenckevort commented 6 months ago

I've refactored the importmap scopes to work as intended (based on the path of the importing script, rather than the path of the document).

FlorianRappl commented 6 months ago

I've refactored the importmap scopes to work as intended (based on the path of the importing script, rather than the path of the document).

Awesome - thanks!

The only thing that keeps me at the moment from merging is that the Linux build (or rather the tests) is failing. I did not have yet the chance to look into it, but it should definitely also work on Linux.

tomvanenckevort commented 6 months ago

Yes, was going to try and have a look at that one today as well.

FlorianRappl commented 6 months ago

You are the man! GJ! 🚀