-
Notifications
You must be signed in to change notification settings - Fork 37
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 Types #112
Fix Types #112
Conversation
@@ -20,4 +20,14 @@ export default document => ${ | |||
.replace(/^(\s+)replaceWith\(([^}]+?)\}/m, '$1/* c8 ignore start */\n$1replaceWith($2}\n$1/* c8 ignore stop */') | |||
.replace(/^(\s+)(["'])use strict\2;/m, '$1$2use strict$2;\n\n$1const { constructor: DocumentFragment } = document.createDocumentFragment();') | |||
.replace(/^[^(]+/, '') | |||
.replace(/\n exports\.Hole = Hole;[\S\s]*return exports;/m, ` |
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.
what has this to do with this MR?
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 fixes type hints of uhtml/init
. Without this, const { html, render } = init(document)
will not have types (or rather be any
). See https://git.envs.net/iacore/data-soup/src/commit/fc8fbb1bb35d071caf01963e53716d95cc4ddf79/server.js#L19
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.
OK ... then we need to do a bit more ... to start with, the exports
object is not needed, we need to replace that too. I now need to maintain manually all exports in this file + it's not clear once we remove tsc
what is typescript dependency doing in here at all ... this feels like a breaking change too ... thoughts?
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.
also this line feels particularly cumbersome render : keyed$1,
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.
uhm ... I think this is landing in here with better logic to let me publish more than just init.js
there: #114
uhm ... I think this is landing in here with better logic to let me publish more than just |
This works. Closes #111.