Test cases from #11 for reference implementation

[guybedford]

I've tried to apply some of the cases mentioned in #11 to the reference implementation and managed to get some odd results for perusal.

Happy to discuss possible solutions here or in #11.

Great work @justinfagnani, the source is a pleasure to read and I was almost worried for a second there you'd managed to elegantly avoid all the edge case issues giving me nothing to complain about :)

[guybedford]

(note that the test results being asserted here are the ones that make the tests pass, not what the expectations should necessarily be)

[guybedford]

I've added a second commit here that adds some basic validations.

I guess the first question is as mentioned in #11 if validations should be performed early (during parsing) or at resolution time as resolution errors.

No stress at all if this implementation is entirely undesired - just trying to move conversations forward on this.

[domenic]

Splitting this up into two commits was really helpful; thanks for doing that.

I do think validations should be performed during parsing, not resolution, if possible.

Your tests seem to rely on the implicit "default path is derived from name" rule a bit, which is somewhat confusing. Because I think the rules should probably be different for package names and for paths. For example I think a package like:

  "moment/../notmoment": {
    "path": "moment-slash-dotdot-slash-notmoment",'
    "main": "index.js"
  }

might be fine. You'd just resolve import "moment/../notmoment" to import "http://foo.com/node_modules/moment-slash-dotdot-slash-notmoment/index.js".

So I think it would be helpful to separate validation of package names (which as I stated in #38, I would rather not do) from validation of paths. This would indeed cause validation errors if you use the implicit path-derived-from-name rule and then have a weird package name. But as written the tests don't make it clear which is being checked right now.

As for how we should validate paths: I wonder if we can lean on the URL spec here. The definitions around https://url.spec.whatwg.org/#url-path-segment-string seem helpful. Those definitions are intentionally stricter than what browsers parse; they are what are considered "valid" URLs, mostly for use by conformance checkers or similar things. But we could use that framework to start out restrictive.

[guybedford]

Sure, I've updated the basic type validations to happen on initialization of the package map. For now I've kept the package name validation to happen in this phase as well, until we can work out how to handle these cases.

I do think it would be better to carefully restrict package names to sensible cases by disallowing internal dot segments and trailing slashes, also in terms of users understanding the difference between package names and URLs. While yes these apply to the "package names as path" cases, they are still interpretations of the package name. Paths can be looser by definition so I do very much see these as package name validations.

For example, if a user were to mistakenly provide a URL-like into a package name (absolute URL, dot seegments). I've added the leading separator and URL case as well as a validation for now to demonstrate the usability here, which I think would be a nice cue for users to understand package names as something different.

If we do decide to go with more loose package name interpretations allowing these things without the stricter validations, that still leaves us with the trailing slash case which is problematic - package-name/ as the package name:

• Should this trigger only for package-name/ and package-name/x and not package-name?
• If so, package-name/x then has to be special-cased to ensure x appears in the output (fb6db5c#diff-a3199dd5a8438364bee42f2d6ffc3826R379)

[domenic]

Thanks, looks great! We'll start here and perhaps get more restrictive over time.