Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(SPEC): add specification of modules #5224

Merged
merged 1 commit into from
Nov 8, 2022
Merged

docs(SPEC): add specification of modules #5224

merged 1 commit into from
Nov 8, 2022

Conversation

nathanielc
Copy link
Contributor

The implementation of this work is still in-progress see #4296

Checklist

Dear Author 👋, the following checks should be completed (or explicitly dismissed) before merging.

  • ✏️ Write a PR description, regardless of triviality, to include the value of this PR
  • 🔗 Reference related issues
  • 🏃 Test cases are included to exercise the new code
  • 🧪 If new packages are being introduced to stdlib, link to Working Group discussion notes and ensure it lands under experimental/
  • 📖 If language features are changing, ensure docs/Spec.md has been updated

Dear Reviewer(s) 👋, you are responsible (among others) for ensuring the completeness and quality of the above before approval.

@nathanielc nathanielc changed the base branch from docs/editions to master September 23, 2022 18:55
docs/SPEC.md Outdated

Examples

import "foo/module" // imports the latest version 0.x or 1.x version of the module
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be ambiguous with standard library modules as it is currently (did you mean the user module "array" or the standard library "array"?). For the existing standard library we can just reserve the names for all existing packages, but that won't work if we want to add a new packages so maybe we also want to add and reserve a std prefix for the standard library?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen things in the JS space where importers use like ~/... to say, "relative to the project's node_modules dir."

It could be useful to have something as a prefix to indicate the importer to use (registry vs stdlib).

docs/SPEC.md Show resolved Hide resolved
| GET | $base/$module/@v/$version.zip | Returns a zip file of the contents of the module at a specific version. |
| GET | $base/$module/@latest | Returns the highest released version, or if no released versions exist the highest pre-release version of the given module in plain text on a single line. |
| GET | $base/$module/@pre | Returns the highest pre-released version of the given module in plain text on a single line. |
| POST | $base/$module/@v/$version | Publish a new version of the module where the POST body contains multipart formdata for the contents of the module. |
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @v delimiter is no longer necessary since a module path is at most two path elements, of the form name/v# where # is a number >= 2. We needed a rule to distinguish module paths from package paths and it so happens we can reuse that rule for the registry API.

Should we remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed and we are going to leave @v in place. We have already implemented it and it helps future proof the API so it makes sense to leave it be.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does POST and @pre work---is there a way to specify that the version you are publishing is release or pre-release?

@nathanielc nathanielc marked this pull request as ready for review November 2, 2022 23:09
@nathanielc nathanielc requested a review from a team as a code owner November 2, 2022 23:09
@nathanielc nathanielc requested review from wolffcm and removed request for a team November 2, 2022 23:09
@nathanielc
Copy link
Contributor Author

NOTE: This specifies two changes to the current implementation of the registry API.

  • the v is required anywhere a version is used
  • the POST no longer expects a .zip extension.

Copy link

@wolffcm wolffcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable to me, but I had some questions about what it means to use @pre.

Example:

@registry("modules", "http://localhost/modules")
@registry("example.com", "https://example.com/api/modules")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The registry attribute needs to appear before the package clause, right? If so, it would be worth mentioning here.

docs/SPEC.md Outdated
An import path follows this grammar:

ImportPath = ModulePath [ "/" PackagePath ] [ Version ] .
ModulePath = PathElement [ "/" PathElement ] [ MajorVersion ] .
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make it clearer to write it like this

Suggested change
ModulePath = PathElement [ "/" PathElement ] [ MajorVersion ] .
ModulePath = RegisteryName [ "/" ModuleName ] [ MajorVersion ] .
RegistryName = PathElement
ModuleName = PathElement


When no version information is provided the latest version of the module is used.
A _minimum_ version may be specified.
An import may also specify the version `pre` which is the most recent pre-release version of the module.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So pre exists to let users get the very latest version, sort of like Rust nightly?

Everything that gets published bumps the version number, right? So I don't see how this would be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am expecting that @pre refers to an explicit pre-release using semantic versioning for pre-release versions. I am expecting that clients (i.e. the UI) will leverage pre-releases to pre-publish or preview a module before doing a real publish of the module. This way users can test their modules without risk of breaking existing consumers.

I imagine a workflow like this:

  1. Publish a module foo
  2. Consume module foo in various other scripts
  3. Decide to improve foo
  4. Publish a pre-release of foo.
  5. Use @pre is specific scripts to ensure its working
  6. Publish a normal release of foo, all other scripts are now upgraded

This work across major versions as well as @pre applies to within a single major version. Meaning there can be a different pre-release for each major version.

| GET | $base/$module/@v/$version.zip | Returns a zip file of the contents of the module at a specific version. |
| GET | $base/$module/@latest | Returns the highest released version, or if no released versions exist the highest pre-release version of the given module in plain text on a single line. |
| GET | $base/$module/@pre | Returns the highest pre-released version of the given module in plain text on a single line. |
| POST | $base/$module/@v/$version | Publish a new version of the module where the POST body contains multipart formdata for the contents of the module. |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does POST and @pre work---is there a way to specify that the version you are publishing is release or pre-release?

@nathanielc
Copy link
Contributor Author

How does POST and @pre work---is there a way to specify that the version you are publishing is release or pre-release?

The pre-release is part of the semantic version spec. Therefore it becomes straightforward to create a pre-release when publishing by specifying a version string that is a pre-release.

@nathanielc nathanielc requested a review from wolffcm November 8, 2022 18:59
Copy link

@wolffcm wolffcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great.

docs/SPEC.md Outdated
@@ -1460,7 +1460,7 @@ Module paths need not be unique across different registries.

#### Registry attribute

The `registry` attribute defines the available registries.
The `registry` attribute defines the available registries and must preceed a package clause.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nit, typo in the word "precede".

The implementation of this work is still in-progress see #4296
@nathanielc nathanielc merged commit 1cdebda into master Nov 8, 2022
@nathanielc nathanielc deleted the docs/modules branch November 8, 2022 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants