Separate typings for node and browser #10
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #10 +/- ## ===================================== Coverage 100% 100% ===================================== Files 3 3 Lines 91 91 Branches 18 18 ===================================== Hits 91 91
Continue to review full report at Codecov.
|
| "main": "node/index.js", | ||
| "module": "node/module.js", | ||
| "typings": "render.d.ts", | ||
| "typings": "src/node.d.ts", |
There was a problem hiding this comment.
Why "typings": "src/node.d.ts"? Why not "typings": "src/index.d.ts"?
There was a problem hiding this comment.
Well, ignore it. src/node.d.ts makes more sense.
There was a problem hiding this comment.
index.{js,d.ts}- common code for node and browser which exports internalrendererandrenderToStringbrowser.{js,d.ts}- client-side code which re-exportsrenderToStringfrom index and exportswithRenderwithtoStringonlynode.{js,d.ts}- server-side code which re-exportsrenderToStringfrom index plusrenderToStreamand also exportswithRenderwithtoStringandtoStream
There was a problem hiding this comment.
But I think that App interface should be in Hyperapp package. In fact app() function has the same definition. It makes more sense for me refers from Hyperapp. Because now you have two App interfaces (both are the same).
There was a problem hiding this comment.
In short, App is the type for one app() function that withRender() will need as param.
There was a problem hiding this comment.
index.{js,d.ts} - common code for node and browser which exports internal renderer and renderToString
browser.{js,d.ts} - client-side code which re-exports renderToString from index and exports withRender with toString only
node.{js,d.ts} - server-side code which re-exports renderToString from index plus renderToStream and also exports withRender with toString and toStream
Great!
There was a problem hiding this comment.
Only a detail about importing.
Here some examples:
// 1. Testing: import { renderToStream, withRender, Render, renderToString } from "../../src/node" // 2. Actually: if I want to import node version import { renderToStream, withRender, Render, renderToString } from "@hyperapp/render/src/node" // 3. Purpose: move typings to root directory import { renderToStream, withRender, Render, renderToString } from "@hyperapp/render/node"Why do you think about the third option?
There was a problem hiding this comment.
Both 2 and 3 options are valid!
They already work in current v2.0.0 and after this PR will work with typings 🎉
Types of changes
Checklist: