Remap ESModule imports at link time in mdoc

[Quafadas]

See #882 , I had to start over and try a different paradigm.

This PR does a lot more than I would have wanted. Explanation to follow, if CI goes green.

[Quafadas]

@tgodzik

Apologies for all the spam today. This is my best shot. I have tests passing on 2.13 and 2.12, but I can't get the 3 series working. I think this means that I don't understand something fundamental about the interaction between 2 & 3 artefacts. Could it be that I would need to ask Arman to publish his library for scala 3?

I had believed, that this would work.
https://github.com/Quafadas/mdoc/blob/1b1f096694b814a19758cd95c4e313926c41d782/mdoc-sbt/src/main/scala/mdoc/MdocPlugin.scala#L160-L161

But it leaves apparently some mismatch with

==> X tests.js.JsCliSuite.basic  0.435s java.lang.NoSuchMethodError: 'byte[] org.scalajs.ir.Names$.org$scalajs$ir$Names$$validateSimpleEncodedName(byte[])'

[tgodzik]

Could it be that I would need to ask Arman to publish his library for scala 3?

That might be the issue or the classpath for Scala 3 is broken still. I can try take a look next week if you're still having difficulties

[Quafadas]

Aha! I have it! Write up in bound!

[Quafadas]

Yes!

[Quafadas]

Finally! Having gotten the class paths right, I was then struck by a bad case of windows-in-CI.

@tgodzik - I'm sorry for all the spam. Here's the writeup - it may be that I need to start over and do this incrementally as this is a bit messy as a big lump change. I do think it's quite cool though so I'm happy I reached the end.

struggled with the class path. In order to figure out the solution, I needed to be able get to a state where the existing JsCliSuite test succeeded, next to mz Remap ESModule test failing. This was problematic because mdoc implicitly assumes that it would find mdoc.properties on the class path.

mdoc/cli/src/main/scala/mdoc/internal/cli/MdocProperties.scala

Line 40 in 3253544

val path = "mdoc.properties"

This means that, to have a new test that configures the CLI differently, I would

1. need a new class path (entire new copy paste SBT unitJS project, how to manage different mdoc.properties generation?, rejected).
2. need a novel testing strategy (definitely last resort)
3. make mdoc.properties explicit and configurable. This is arguably a useful feature in it's own right.

Most of the change in the PR end up being to make option 3 viable.

Most changes are then forced because mdoc main branch parses the CLI args after (!) it reads the mdoc.properties file that mdoc assumes it finds on the class path. In particular, much of the test setup is based around that assumption.

To make this work, I had to change that order. i.e. parse the CLI args first. Then go search for a configurably named (default = mdoc.properties) file.

This meant making "mdoc.properties" explicit in a few places in the codebase (tests in particular). That may make this change controversial. Also : required adding braces in many places, which then required reformatting lots of files. Yey?

At this point, I could then generate a new es.properties next to the default mdoc.properties in the plugin. Both properties files are then concurrently on the class path, but selected by the CLI.

This allowed for more than one test in the suite. After that the scientific method bailed me out of needing to actually understand class paths :-), and the PR (finally) went green with two new integration tests for ESModules.

It's a lot more than I intended. I'm not sure how to best get it comfortable to merge. I also need to write some docs. Happy to add to this PR... or a separate follow up.

[tgodzik]

The PR doesn't look to crazy and I think the situation with mdoc.properties file could be improved. I think this looks ok

[Quafadas]

Okay - thank you - that is encouraging! I've rebased - hopefully correctly.

I'm not trying to pressure you either - perhaps we leave it open for another couple of weeks, and if it doesn't attract more feedback one way or another, then call pulling the trigger or close it? Mostly I would wish to avoid having it hanging around as summer ends.

[tgodzik]

I'll wait until the next week then.

[tgodzik]

Any chance you could reopen this PR with only the needed changes? It's seems the commit history is too long and there unrelated changes showing up in the PR.

[Quafadas]

Any chance you could reopen this PR with only the needed changes? It's seems the commit history is too long and there unrelated changes showing up in the PR.

I guess I need to. This PR starts from the end and worked back to the start point. I think what I should do, is open a series of PRs which start at the main branch, and work forward towards this. I think they are;

1. make mdoc.properties configurable, so that we can have more than one classpath arrangement in the test suite
2. Introduce the changes which make mdocJs consume ESModules
3. Introduce the remapping function

Unfortunately, it's going to take more time - I have more flexibility over summer than now... but it's on my list. Thankyou again for looking.

[tgodzik]

It's fine for me to just tidy up the PR, I can do it otherwise.

[Quafadas]

I think that's everything? I've done all that I could see.

[tgodzik]

Let's merge it! Thanks for working on it!

[Quafadas]

@tgodzik Thankyou 🙏 !

[Quafadas]

I've tested this PR as much as I can. As far as I can tell, it works, it's just astonishingly tricky to configure correctly.

Here's a repo that loads shoelace as an ESModule inside a laminar app, and loads icons on request.

This repository leaves some breadcrumbs, if anyone was interested.
https://github.com/Quafadas/mdoc_test