Skip to content

feat: native streams#1365

Merged
Sam Gammon (sgammon) merged 9 commits intomainfrom
feat/native-streams
May 28, 2025
Merged

feat: native streams#1365
Sam Gammon (sgammon) merged 9 commits intomainfrom
feat/native-streams

Conversation

@darvld
Copy link
Copy Markdown
Member

@darvld Dario Valdespino (darvld) commented Apr 12, 2025

Ready for review Powered by Pull Request Badge

Summary

Introduces a pure-Kotlin implementation of the Web Streams standard, meant to replace the polyfills currently in place.

The implementation is functional but far from optimal, as it was made to be as close as possible to the specification, which resulted in JavaScript-style code that does not fully utilize the capabilities of Kotlin. In the future, a more optimal and streamlined (pun intended) implementation will be added, likely using coroutine primitives such as channels to avoid the current boilerplate.

  • Interface definitions for the streams API
  • Deduplicate stream API interfaces
  • Readable streams
    • Default controller/reader
    • BYOB controller/reader
    • Guest ReadableStreamSource wrapper
  • Writable streams
    • Default controller/writer
    • Guest WritableStreamSink wrapper
  • Transform streams
    • Default controller
    • Guest TransformStreamTransformer wrapper
  • Basic test suite
  • Disable streams polyfill

Note

A companion PR at runtime/382 removes the polyfill from the runtime image.

@darvld Dario Valdespino (darvld) added the 🚧 WIP Works-in-progress. Blocks merge label Apr 12, 2025
@sgammon Sam Gammon (sgammon) changed the title feat: native streams [wip] feat: native streams Apr 12, 2025
Copy link
Copy Markdown
Member

@sgammon Sam Gammon (sgammon) left a comment

Choose a reason for hiding this comment

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

docs and approach look solid

@sgammon
Copy link
Copy Markdown
Member

CI failures

/home/runner/work/elide/elide/packages/graalvm/src/main/kotlin/elide/runtime/gvm/internals/intrinsics/js/webstreams/ReadableDefaultStream.kt:104:16: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled. [TooGenericExceptionCaught]

Feel free to run :packages:graalvm:detektBaseline

Execution failed for task ':packages:graalvm:spotlessKotlinCheck'.

Format with :packages:graalvm:spotlessApply, modify the CI job to drop this until it's non-draft. Honestly I might just remove spotless anyway.

@sgammon Sam Gammon (sgammon) linked an issue Apr 12, 2025 that may be closed by this pull request
@darvld Dario Valdespino (darvld) force-pushed the feat/native-streams branch 3 times, most recently from 727b582 to b3e8ed0 Compare May 13, 2025 01:26
@codecov
Copy link
Copy Markdown

codecov bot commented May 24, 2025

Codecov Report

Attention: Patch coverage is 37.01380% with 1004 lines in your changes missing coverage. Please review.

Project coverage is 42.82%. Comparing base (a30547a) to head (d41a8ca).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...als/intrinsics/js/webstreams/ReadableByteStream.kt 28.80% 282 Missing and 27 partials ⚠️
.../intrinsics/js/webstreams/WritableDefaultStream.kt 30.57% 140 Missing and 28 partials ⚠️
.../intrinsics/js/webstreams/ReadableDefaultStream.kt 46.62% 60 Missing and 19 partials ⚠️
...intrinsics/js/webstreams/TransformDefaultStream.kt 31.68% 59 Missing and 10 partials ⚠️
...m/internals/intrinsics/js/webstreams/StreamPipe.kt 27.38% 53 Missing and 8 partials ⚠️
...intrinsics/js/webstreams/ReadableStreamWrappers.kt 0.00% 58 Missing ⚠️
...als/intrinsics/js/webstreams/ReadableStreamBase.kt 50.60% 28 Missing and 13 partials ⚠️
...de/runtime/intrinsics/js/stream/QueuingStrategy.kt 17.64% 28 Missing ⚠️
.../js/webstreams/WritableStreamDefaultWriterToken.kt 53.44% 22 Missing and 5 partials ⚠️
...in/kotlin/elide/runtime/intrinsics/js/JsPromise.kt 58.00% 16 Missing and 5 partials ⚠️
... and 21 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1365      +/-   ##
==========================================
- Coverage   43.13%   42.82%   -0.32%     
==========================================
  Files         647      676      +29     
  Lines       29157    30631    +1474     
  Branches     3928     4258     +330     
==========================================
+ Hits        12577    13117     +540     
- Misses      15181    15964     +783     
- Partials     1399     1550     +151     
Flag Coverage Δ
jvm 42.82% <37.01%> (-0.32%) ⬇️
lib 42.82% <37.01%> (-0.32%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ntrinsics/js/webstreams/ReadableStreamIntrinsic.kt 100.00% <100.00%> (+75.00%) ⬆️
...ics/js/webstreams/ReadableStreamReaderTokenBase.kt 100.00% <100.00%> (ø)
...trinsics/js/webstreams/TransformStreamIntrinsic.kt 100.00% <100.00%> (ø)
...ntrinsics/js/webstreams/WritableStreamIntrinsic.kt 100.00% <100.00%> (ø)
...time/gvm/intrinsics/BuildTimeIntrinsicsResolver.kt 100.00% <100.00%> (ø)
...ain/kotlin/elide/runtime/node/fs/NodeFilesystem.kt 68.07% <100.00%> (ø)
...in/elide/runtime/plugins/AbstractLanguagePlugin.kt 37.50% <0.00%> (ø)
...nsics/js/stream/WritableStreamDefaultController.kt 0.00% <0.00%> (ø)
...tlin/elide/runtime/intrinsics/ai/ElideLLMModule.kt 27.38% <0.00%> (ø)
...e/intrinsics/js/stream/ReadableStreamBYOBReader.kt 50.00% <50.00%> (ø)
... and 27 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a30547a...d41a8ca. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@darvld Dario Valdespino (darvld) removed the 🚧 WIP Works-in-progress. Blocks merge label May 27, 2025
@darvld Dario Valdespino (darvld) marked this pull request as ready for review May 27, 2025 19:09
@darvld Dario Valdespino (darvld) changed the title [wip] feat: native streams feat: native streams May 27, 2025
@sgammon Sam Gammon (sgammon) added feature Large PRs or issues with full-blown features lang:javascript Issues relating to JavaScript labels May 27, 2025
@sgammon Sam Gammon (sgammon) moved this to Done in Elide May 27, 2025
@darvld Dario Valdespino (darvld) force-pushed the feat/native-streams branch 3 times, most recently from d2a0195 to 29b9e5d Compare May 28, 2025 16:41
Dario Valdespino (darvld) and others added 5 commits May 28, 2025 12:46
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Sam Gammon <sam@elide.dev>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
Signed-off-by: Dario Valdespino <dvaldespino00@gmail.com>
@sgammon Sam Gammon (sgammon) merged commit 01cbf9c into main May 28, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Large PRs or issues with full-blown features lang:javascript Issues relating to JavaScript

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Native web steams

2 participants