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: deno types #266

Merged
merged 4 commits into from
Apr 1, 2022
Merged

fix: deno types #266

merged 4 commits into from
Apr 1, 2022

Conversation

mathe42
Copy link
Contributor

@mathe42 mathe42 commented Mar 30, 2022

This PR fixes the deno types. There was a mistake (so not all dependencies were bundled).
The changes to the rollup config:

  1. tell dts to include external types
  2. renderChunk code to replace first line of .d.ts file with a custom line (From node type include to type Buffer = ArrayBufferLike wich is the complete typing of node stuff we need.)

Sorry that was my mistake in 8ea51ac didn't test it properly then.

@jjhbw
Copy link
Collaborator

jjhbw commented Apr 1, 2022

Alright, thanks! So as I understand it, this PR only changes the bundled.d.ts file like in the below diff.

I also noticed this warning appeared in my shell after yarn compile:

./lib/index.d.ts → ./lib/bundled.d.ts...
(!) Unresolved dependencies
https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency
stream (imported by node_modules/@types/sax/index.d.ts)

Is that something to worry about?

How can I test this? Do you have some Deno example code that I can run?

image

@mathe42
Copy link
Contributor Author

mathe42 commented Apr 1, 2022

Here is an example https://github.com/mathe42/deno-docx-templates. Note that you have to set noSanbox to true as the vm-browser module expects IFrame etc. to exists. Added it to readme

@mathe42
Copy link
Contributor Author

mathe42 commented Apr 1, 2022

The external dependency warning is ok as all types that depend on it are removed from final bundle. I edited the config so this warning is removed.

@jjhbw jjhbw merged commit ac8822b into guigrpa:master Apr 1, 2022
@mathe42 mathe42 deleted the patch-2 branch April 1, 2022 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants