Package main sugar

[guybedford]

This adds the package main sugar case from #3, as well as testing some URL main cases which aren't currently tested.

Note if these types of feature PRs are too early, it's no problem to close and focus purely on the validation and test coverage diffs. This stuff is easy to add anytime.

[domenic]

This doesn't seem like it's implementing #3, but instead #4. We should be sure to note that correctly in the commit message.

I think the main thing we should also include here is what happens for submodules? Let us discuss that in #4 to keep things a bit focused. But I'd like to include tests for whatever we decide before merging this feature.

[domenic]

I guess we already know what the submodule behavior should be; I got confused. But, I'd like us to add a few tests for it :)

[guybedford]

Sure, I've made those changes. Not the submodule behaviour will depend on #43 here.

[domenic]

It seems like the type system is complaining now; not sure how to fix exactly.

We should also add tests for empty objects to this PR, I think.

[guybedford]

Another question that comes up here, is should subpaths be supported for string mains?

{
  packages: {
    pkg: '/path/to/main.js'
  }
}

In such a case we could:

• Set the "path" to dirname('/path/to/main.js')
• Set the "path" to undefined and support the subpath being "/pkg/[subpath]"
• Throw if trying to import a submodule for a package that is defined to be a string.

I'm tempted to say throwing seems the most well-defined, but it does involve treating packages as strings or objects deeper down into the codebase, unless we add a flag property to package "isMainSugar: true" or similar.

[guybedford]

(I've got the typing fixes and test cases ready to go if we can clarify some of the above first)

[domenic]

Hmm my take is we'd do the transformations in this way:

{
  packages: {
    pkg: '/path/to/main.js'
  }
}

->

{
  packages: {
    pkg: {
       main: '/path/to/main.js'
    }
  }
}

->

{
  packages: {
    pkg: {
       path: 'pkg'
       main: '/path/to/main.js'
    }
  }
}

WDYT?

[guybedford]

My worry here is the unintuitive split between URL mains and package names as paths.

For example, perhaps few would expect this to work when it would:

{
  path_prefix: 'node_modules'
  packages: {
    pkg: 'index.js'
  }
}

where,

• import 'pkg' -> node_modules/pkg/index.js
• import 'pkg/x' -> node_modules/pkg/x

[ljharb]

I’m not sure who would be using package name maps that would be entirely unfamiliar with node’s module system, or, who wouldn’t be able to rely on copious documentation explaining how name maps works.

[domenic]

@guybedford Yeah, I see the problem. It seems like this is inherent in any solution that has implicit paths though. Or maybe it's just worse because of the string-valued version?

Maybe let's step back a bit.

{
  "package1": {
    "path": "path1",
    "main": "main1.js"
  },
  "package2": {
    "main": "main2.js"
  },
  "package3": {
    "path": "path3"
  },
  "package4": { },
  "package5": "main5.js"
}

Then, according to our current plan, I think these are the results, right?

• "package1" -> "https://example.com/path1/main1.js"
• "package1/sub" -> "https://example.com/path1/sub"
• "package2" -> "https://example.com/package2/main2.js"
• "package2/sub" -> "https://example.com/package2/sub"
• "package3" -> PARSE ERROR missing main
• "package3/sub" -> PARSE ERROR missing main
• "package4" -> PARSE ERROR missing main
• "package4/sub" -> PARSE ERROR missing main
• "package5" -> "https://example.com/package5/main5.js"
• "package5/sub" -> "https://example.com/package5/sub"

(For the parse errors, actually just including those in the name map breaks the whole name map. But for illustrative purposes I listed these all separately.)

Which of these seem like maybe we're making the wrong decision?

[guybedford]

The main throwing errors are currently left out, I've added these in #45.

To go over the cases:

{
  "package1": {
    "path": "path1",
    "main": "main1.js"
  },
  "package2": {
    "main": "main2.js"
  },
  "package3": {
    "path": "path3"
  },
  "package4": { },
  "package5": "main5.js"
}

Then the corrected cases are:

1. "package1" -> "https://example.com/path1/main1.js"
2. "package1/sub" -> "https://example.com/path1/sub"
3. "package2" -> "https://example.com/package2/main2.js"
4. "package2/sub" -> "https://example.com/package2/sub"
5. "package3" -> PARSE ERROR missing main
6. "package3/sub" -> PARSE ERROR missing main "https://example.com/package3/path3/sub"
7. "package4" -> PARSE ERROR missing main
8. "package4/sub" -> PARSE ERROR missing main "https://example.com/package4/sub"
9. "package5" -> "https://example.com/package5/main5.js"
10. "package5/sub" -> "https://example.com/package5/sub" - this is currently undefined behaviour depending on how this PR works out

All of the cases do seem mostly sensible to me though, it's perhaps some of the main as a URL edge cases that get odd, but it's a super tough design space.

[domenic]

OK, in that case I think we should either:

• Abandon this sugar for now as we're not sure about it yet, and we can always add it later; or
• Go with the original plan, i.e. 'Set the "path" to undefined and support the subpath being "/pkg/[subpath]"'.

[guybedford]

I've updated to the original case here of setting path to undefined. I do think this main sugar is important to have for simple workflows and the edge case scenarios, which are growing on me, apply equally well to URL mains without a path in the non-sugar case anyway.

[domenic]

Looks great, thanks for working through all this.