Various quality of life improvements to the CLI#4414
Conversation
Allow an expression to be passed from the arguments (using -e). This makes it easier to just run a single expression and returning. Additionally, if the value returned by a _file_ is undefined, don't show it. It gets confusing to see a list of "undefined" on the command line when running a bunch of modules in batch. Another change is merging the error printing code so the output is more consistent. As well, I added distinction between an error thrown from a statement versus from a job, so the user can see where it's coming from. Added support for timeout and generic jobs in the CLI. Now we can use setTimeout or any generic jobs. The CLI will always wait for all jobs to be completed before exiting, which makes setInterval a bit awkward if it has no end conditin. Finally, fixed a bug in setTimeout where errors were not propagated properly.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4414 +/- ##
==========================================
+ Coverage 47.24% 50.69% +3.44%
==========================================
Files 476 501 +25
Lines 46892 51236 +4344
==========================================
+ Hits 22154 25973 +3819
- Misses 24738 25263 +525 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cli/src/main.rs
Outdated
| self.promise_jobs.borrow().is_empty() | ||
| && self.async_jobs.borrow().is_empty() | ||
| && self.timeout_jobs.borrow().is_empty() | ||
| && self.generic_jobs.borrow().is_empty() |
There was a problem hiding this comment.
This works for our current usage of the context since we don't use multiple contexts, but if we decide to add web workers in the future, it will break. Not calling context.has_pending_context_jobs() and context.enqueue_resolved_context_jobs() will completely skip resolving calls to Atomics.notify.
There was a problem hiding this comment.
I'm about to add workers but they will use a separate Context, not just a new Realm. That's the only way to make them multi-threaded.
I'll add the context stuff, but I'm curious why aren't those already enqueued?
There was a problem hiding this comment.
Seems like those could be AsyncJobs.
There was a problem hiding this comment.
I'll add the context stuff, but I'm curious why aren't those already enqueued?
Because having the context itself enqueue its own pending notify jobs was the simpler way to enable multi-threaded job enqueuing. The alternative was to modify the API for JobExecutor to take an Arc, but we would have to deal with Rust's strict semantics on non-Send, non-Sync jobs, and implementors of JobExecutor would need to use RwLock or some other multi-threaded state to keep track of all the jobs.
There was a problem hiding this comment.
As a whole, it seemed much simpler to have a multi-threaded flag on the context to enqueue its notify jobs, and have enqueue_resolved_context_jobs check that flag to see if there are pending notify jobs.
There was a problem hiding this comment.
Seems like those could be AsyncJobs
Ohhhhh! Yeah, this made me realize it is possible to use an async oneshot channel to resolve an async job that does the same, which would nicely avoid having to manually track all futex waiters in the context
3 participants
Allow an expression to be passed from the arguments (using -e). This makes it easier to just run a single expression and returning.
Additionally, if the value returned by a file is undefined, don't show it. It gets confusing to see a list of "undefined" on the command line when running a bunch of modules in batch.
Another change is merging the error printing code so the output is more consistent. As well, I added distinction between an error thrown from a statement versus from a job, so the user can see where it's coming from.
Added support for timeout and generic jobs in the CLI. Now we can use setTimeout or any generic jobs. The CLI will always wait for all jobs to be completed before exiting, which makes setInterval a bit awkward if it has no end conditin.
Finally, fixed a bug in setTimeout where errors were not propagated properly.