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

fix: typescript support #968

Merged
merged 1 commit into from
Jun 4, 2024
Merged

fix: typescript support #968

merged 1 commit into from
Jun 4, 2024

Conversation

sgammon
Copy link
Member

@sgammon sgammon commented Jun 4, 2024

Ready for review Powered by Pull Request Badge

Summary

Fixes TypeScript support from the entrypoint; fixes the TypeScript module patcher which allows for TypeScript-native imports.

Changelog

  • fix: typescript compiler and js realm/context use
  • fix: invocation of typescript from entrypoint
  • fix: module loader requires access to disk

- fix: typescript compiler and js realm/context use
- fix: invocation of typescript from entrypoint
- fix: module loader requires access to disk

Signed-off-by: Sam Gammon <sam@elide.ventures>
@sgammon sgammon added bug Something isn't working lang:typescript Language features relating to TypeScript labels Jun 4, 2024
@sgammon sgammon added this to the Release R6: Alpha 10 milestone Jun 4, 2024
@sgammon sgammon self-assigned this Jun 4, 2024
Copy link

sonarcloud bot commented Jun 4, 2024

Copy link

codecov bot commented Jun 4, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

Project coverage is 54.87%. Comparing base (7b356b2) to head (906651e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #968      +/-   ##
==========================================
+ Coverage   54.81%   54.87%   +0.05%     
==========================================
  Files         391      391              
  Lines       11537    11547      +10     
  Branches     1918     1918              
==========================================
+ Hits         6324     6336      +12     
+ Misses       4661     4659       -2     
  Partials      552      552              
Flag Coverage Δ
gradle 54.87% <94.73%> (+0.05%) ⬆️
jvm 54.87% <94.73%> (+0.05%) ⬆️
lib 54.87% <94.73%> (+0.05%) ⬆️
plugin 54.87% <94.73%> (+0.05%) ⬆️

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

Files Coverage Δ
...de/runtime/lang/typescript/TypeScriptLanguage.java 92.85% <100.00%> (+0.85%) ⬆️
.../lang/typescript/internals/TypeScriptCompiler.java 57.50% <100.00%> (+10.62%) ⬆️
...lin/elide/runtime/plugins/typescript/TypeScript.kt 0.00% <ø> (ø)
...de/runtime/core/internals/graalvm/GraalVMEngine.kt 76.47% <100.00%> (ø)
...elide/runtime/gvm/internals/sqlite/SqliteModule.kt 83.49% <100.00%> (+0.59%) ⬆️
...otlin/elide/runtime/plugins/js/JavaScriptConfig.kt 96.29% <100.00%> (ø)
...main/kotlin/elide/runtime/plugins/js/JavaScript.kt 94.68% <50.00%> (-1.07%) ⬇️

... 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 7b356b2...906651e. Read the comment docs.

Copy link
Member

@darvld darvld left a comment

Choose a reason for hiding this comment

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

Looks good, just a comment

// on SVM, we should load the library directly using our own tools; this is because sqlite3 may be built into the
// binary statically, and loaded via our own facilities at build-time.
if (ImageInfo.inImageCode()) NativeLibraries.resolve(SQLITE3_LIBRARY) {
org.sqlite.SQLiteJDBCLoader.initialize()
} else {
// otherwise, on JVM, we should load the library through regular SQLite3 JDBC mechanisms; this will unpack the
// library to a temporary location, load it, and then clean up after itself.
NativeLibraries.resolve(SQLITE3_LIBRARY) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this change meant to be part of this PR? Seems related to SQLite instead.

Copy link
Member Author

@sgammon sgammon Jun 4, 2024

Choose a reason for hiding this comment

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

Yes, it's related -- fixes for native libs

@sgammon sgammon merged commit a9a17c5 into main Jun 4, 2024
22 checks passed
@sgammon sgammon deleted the fix/typescript branch June 4, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lang:typescript Language features relating to TypeScript
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

TypeScript won't run
2 participants