fix: cleanup build #104
fix: cleanup build #104
Conversation
src/index.ts
Outdated
| ); | ||
|
|
||
| const defaultParse = <$Body>(response: Response): Promise<$Body> => | ||
| const defaultParse = (response: Response): Promise<any> => |
There was a problem hiding this comment.
Hi @dword-design what was the reason to remove these types?
There was a problem hiding this comment.
Reason was that the typing in the current TypeScript reason leads to errors. response.json() returns just any (but which is not optimal imho), and response.text() returns a string. The problem is that response.text() contradicts with $Body. If that special handling wouldn't be there, Promise<$Body> would work. Setting Promise<$Body | string> leads to TypeScript errors when building.
Setting it to Promise<any> is not good though, so better have the types clean.
| "scripts": { | ||
| "test": "jest", | ||
| "build": "webpack && tsc", | ||
| "build": "tsc", |
There was a problem hiding this comment.
Hey @dword-design thanks for the contribution! We're able to remove Webpack because we no longer need to transpile now that we're on ESM?
There was a problem hiding this comment.
Yeah. I'd say Node.js runs right away and for the browser it should be possible to either use a bundler for the end-user project or import ESM. I tried it for defaults which only has an ESM export (and 29M downloads) and it works:
<!-- index.html -->
<script type="module">
import defaults from "https://esm.sh/defaults";
console.log(defaults({ a: 2 }, { a: 1, b: 2 }));
</script>| @@ -1,2 +0,0 @@ | |||
| declare module 'parse-link-header'; | |||
There was a problem hiding this comment.
Hi @dword-design what's the reason this type isn't needed anymore?
There was a problem hiding this comment.
Why would we need it? It's imported in src/index.ts.
2 participants
I got this error the last time I used the lib in a project:
So I decided to clean up the build and to migrate to ESM.