-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
JS export #17296
JS export #17296
Conversation
Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
@@ -110,3 +110,15 @@ proc parse*(l: JsonLib, s: cstring): JsRoot {.importcpp.} | |||
since (1, 5): | |||
func debugger*() {.importjs: "debugger@".} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is "debugger@"
documented and what does it mean?
manual mentions "createDevice(@)"
but not "createDevice@"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ cat example.nim
func debugger*() {.importjs: "debugger".}
debugger()
$ nim js example.nim
Error: `importjs` for routines requires a pattern
$
🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- but shouldn't it be
"debugger(@)" ?
"debugger@"` seems wrong and may (in future) turn into a CT error - in future work we should support
importjs: "debugger"
though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"debugger(@)"
emits debugger()
but must be debugger
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you document this in a followup PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't you use importc: "debugger"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a small bug on importjs tho. 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i added a note in nim-lang/RFCs#315 so we don't lose track
Co-authored-by: Timothee Cour <timothee.cour2@gmail.com>
Co-authored-by: Timothee Cour <timothee.cour2@gmail.com>
Co-authored-by: Timothee Cour <timothee.cour2@gmail.com>
Gitlab is 500'ing in the CI. 😐 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I guess we also need jsImport
in future work then?
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import)
docs look a bit funny with the runnableExamples at the top, but that'll be fixed when this regression is fixed: #16993
https://github.com/nepeckman/jsExport.nim might be worth looking into for some ideas btw |
1 similar comment
https://github.com/nepeckman/jsExport.nim might be worth looking into for some ideas btw |
@Yardanico has a good point, if we're offering it in stdlib, might as well make it as good as can be; but then it should be it's own module IMO; please check with @Araq first whether he'd accept a well designed |
We still have Fusion as the staging area... |
jscore.jsexport
.