Skip to content

Remove orphan temporary files; support Deno and Bun#1186

Closed
guest271314 wants to merge 3 commits intobytecodealliance:mainfrom
guest271314:guest271314-patch-1
Closed

Remove orphan temporary files; support Deno and Bun#1186
guest271314 wants to merge 3 commits intobytecodealliance:mainfrom
guest271314:guest271314-patch-1

Conversation

@guest271314
Copy link
Copy Markdown
Contributor

Fixes #1185

Description of the change

Remove orphan temporary files; support Deno and Bun

Why am I making this change?

The code in the current example results in orphan files being left in temporary directory. Support Deno and Bun using the same code. Eventually support more JavaScript runtimes, such as txiki.js.

Checklist

  • I've updated the default plugin import namespace and incremented the major version of javy-plugin-api if the QuickJS bytecode has changed.
  • I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli, javy-plugin, and javy-plugin-processing do not require updating CHANGELOG files.
  • I've updated the relevant crate versions if necessary. Versioning policy for library crates
  • I've updated documentation including crate documentation if necessary.

Copy link
Copy Markdown
Collaborator

@jeffcharles jeffcharles left a comment

Choose a reason for hiding this comment

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

I appreciate this took some time to put together but this is way too much code to review and maintain for a docs page. I also want to keep the JS host example code simple so it's easy for a reader to understand, it's not intended to be production-ready code. I'm okay with changes to broaden support for JS runtimes that have existing WASI support and okay with changes to clean up the temp files. This is a more minor point but I'd also prefer if we kept using the async APIs and not the sync ones.

@guest271314
Copy link
Copy Markdown
Contributor Author

I appreciate this took some time to put together but this is way too much code to review and maintain for a docs page.

I will maintain the code.

Documentation should be correct.

I also want to keep the JS host example code simple so it's easy for a reader to understand, it's not intended to be production-ready code.

If somebody/anybody winds up here in Javy world, chances are it ain't they're first rodeo. JavaScript to WebAssembly toolchain is non-trivial. This is one of, if not the only library doing that. I expect folks running javy are apt in at least JavaScript, if not Rust and WebAssembly tooling, too.

I'm okay with changes to broaden support for JS runtimes that have existing WASI support and okay with changes to clean up the temp files. This is a more minor point but I'd also prefer if we kept using the async APIs and not the sync ones.

I can change the sync methods to async methods.

Basically all JavaScript runtims support WASI. It just takes somebody to write up the code - because every single WASI implementation I've seen is different - it's host defined; there's no canonical coe or implementation.

@guest271314
Copy link
Copy Markdown
Contributor Author

@jeffcharles

but this is way too much code to review and maintain for a docs page.

I think the same general analysis and observation must also apply to node:wasi. Except we can't really get at and modify that code without going through uvwasi, which in reality winds up being even more code. So, 6 of one, half-dozen of the other.

@guest271314
Copy link
Copy Markdown
Contributor Author

but this is way too much code to review and maintain for a docs page.

@jeffcharles I removed all of the exports Javy doesn't expect from WASIP1 host implementations. Only the imports Javy uses (throws when not callable) are included.

@jeffcharles
Copy link
Copy Markdown
Collaborator

As I mentioned in my prior feedback, please remove wasi.js from this pull request. This project is not the place to define a host runtime agnostic WASI implementation. There is value in providing that implementation, but please provide it elsewhere.

@guest271314
Copy link
Copy Markdown
Contributor Author

So, just add rm() to get rid of those orphan files, and leave everything else, as is?

@guest271314
Copy link
Copy Markdown
Contributor Author

@jeffcharles

As I mentioned in my prior feedback, please remove wasi.js from this pull request. This project is not the place to define a host runtime agnostic WASI implementation.

Then where would that be? Parent WebAssembly/WASI repository?

Right now, since there is no canonical implementation, using WASI at all in JavaScript is all third-party hit and miss, and maintainer preference.

@jeffcharles
Copy link
Copy Markdown
Collaborator

Maybe ECMA's TC39 or TC55?

@guest271314
Copy link
Copy Markdown
Contributor Author

Banned from both. ECMA-262 doesn't specifiy I/O at all. Neither does the Winter CG folks. I brought up the lack of STDIO in JavaScript. I guess they are busy with other stuff. I think WebAssembly/WASI should spell it out WebAssembly/WASI#914. It's their idea. Deno, Node, txiki.js, and other JavaScript runtimes could support this, coming from the horse's mouth. Then there wouldn't be the Node.js and reluctance for others' gear - even though that gear works, folks evidently would rather have no functionality for runtimes other than node without some kinda branding/label of an organization. It'd be canonical coming from WebAssembly/WASI. Otherwsie, it will just continue to be arbitrary; from runtime to runtime, library to library; micking the state of I/O in JavaScript right now.

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.

Node.js example leaves orphan files in temporary directory; support Deno and Bun in example

2 participants