Skip to content

Conversation

@BrianJDrake
Copy link
Contributor

The last commit of #191 reinforced that scripts output by jco transpile can depend on the JS module preview2-shim. This isn't really clear from the current README. Let's fix that, and take the opportunity to rewrite the whole README for clarity.

To do:

  • Rewrite whole README for clarity
  • Clarify dependencies of jco itself (it seems to require Wasm, at least)
  • Clarify dependencies of scripts output by jco transpile

@BrianJDrake BrianJDrake marked this pull request as draft October 17, 2023 10:45
@yoshuawuyts
Copy link
Member

Hi there, thanks for starting this PR. I wanted to share some initial thoughts:

Can we please move any new instructions about how to contribute to the project (including dependencies) to the docs. If you want to clarify how things work internally, maybe it would be good to setup a new architecture.md file? I think this would be really useful, but adding that to the README might not be the right place for that.

Can you elaborate which things you don't find clear, and why you believe they must be rewritten? Most of the changes I'm seeing in this PR so far seem like they mostly come down to preference. It would be good to motivate this more strongly.

Thank you!

@BrianJDrake
Copy link
Contributor Author

Can we please move any new instructions about how to contribute to the project (including dependencies) to the docs.

That was my intention (#184).

I am referring here to dependencies for using jco. The README suggests that it runs in any JS environment. To me, 'any JS environment' implies 'no Wasm'. But using jco requires Wasm.

If you want to clarify how things work internally, maybe it would be good to setup a new architecture.md file?

That belongs in a separate discussion. I don't have enough experience with this project (or enough experience with Rust) to take on something like that.

Can you elaborate which things you don't find clear, and why you believe they must be rewritten?

I find the README to be messy overall. It annoys me that the short description at the top (just under the main heading) is different to the repository description. It annoys me more that the rest of the README is also inconsistent: the features summary in the overview looks different to the rest of the README, and the three sets of detailed documentation (the code block showing CLI usage, the subsections for individual CLI commands and the API section) are all different.

It isn't clear from a glance that jco supports JavaScript both as a language to be called from (JavaScript code can call jco) and as a language for authoring components (jco can manipulate JavaScript code, in the sense of converting between that and Wasm components).

Then there's transpilation. The API section just says:

Transpile a Component to JS.

That's not really accurate.

The code block showing CLI usage says:

Transpile a WebAssembly Component to JS + core Wasm for JavaScript execution

That's better, but still says nothing about the WASI shim. Only a careful reading of the transpilation subsection reveals that information. And even there it's not clear:

Components relying on WASI bindings will contain external WASI imports, which are automatically updated to the @bytecodealliance/preview-shim package.

The first part of this sentence seems to be referring to the same thing twice under different terms ('WASI bindings' and 'WASI imports'). Actually, that would be 'external WASI imports', with an unnecessary extra word. The second part of the sentence says that these imports are 'updated'; that word is not clear. And, of course, the package name is wrong.

@yoshuawuyts yoshuawuyts added the documentation Improvements or additions to documentation label Oct 20, 2023
@guybedford
Copy link
Collaborator

I've added some readme reworking and also included the security mention here in the project description. If you feel there are other changes we could make, specific suggestions very much welcome further.

@guybedford guybedford closed this Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants