Add types/tokenizers.d.ts for Angular/Vite/Rollup compatibility#32
Conversation
nico-martin
left a comment
There was a problem hiding this comment.
Hi @dpfens, thank you so much for looking into this!
My main concern is that you replace all alias imports with relative imports. That not necessarily bad, but I'd rather have the aliases in the source code. Also I don't think it solves the underlying problem.
I've looked into the problem and it seems like in some places types/static/tokenizer.d.ts is referenced, but this file does not exist. Whether imported via an alias or as a relative path (tsc-alias has also worked correctly for me with bundler).
The reason is that tsc does not copy src/static/tokenizer.d.ts to the types folder. Therefore other code refrencing types/static/tokenizer.d.ts. I think the solution would be to rename src/static/tokenizer.d.ts to src/static/tokenizer.ts so its treated as a ts file in tsc.
Could you try that and see if it solves your problem?
- Keep the source aliases
- Rename
src/static/tokenizer.d.tstosrc/static/tokenizer.tssotscemitstypes/static/tokenizer.d.ts. - Export the first
SpecialTokeninterface in that file so the duplicateSpecialTokendeclarations can merge cleanly in a.tsfile. - Keep
tsc-alias, but change the@utilspath fromsrc/utils/index.tstosrc/utils/index. With the.tsextension removed,tsc-aliasrewrites@utilscorrectly.
nico-martin
left a comment
There was a problem hiding this comment.
looks good to me now 🤗
xenova
left a comment
There was a problem hiding this comment.
Thanks for the PR! It's in a very clean state, and I am happy to merge the implementation.
One question though: I haven't seen other libraries use "bundler" exports before
"bundler": {
"import": "./dist/tokenizers.mjs"
},
@dpfens Could you point me to other libraries which do this and/or the docs/part of angular which accepts this?
Thanks!
@xenova I mixed this up with a That change has been dropped. |
Thanks for updating 👍 and can you confirm that with these current changes, it fixes the issues you had for angular? If so, I'm happy to merge. |
@xenova Yes, I just confirmed this fixes the # in this tokenizers.js branch
npm pack
# in my project
npm install ~/path/to//bundler-support-branch/huggingface-tokenizers-0.1.3.tgz
npm run buildand the compilation/missing type issues were fixed. Thank you again for your help with this change. |
Closes #31
TypeScript doesn't rewrite path aliases in .d.ts output.
tsc-aliashandles this as a post-processing step, but it doesn't work under "moduleResolution": "bundler" (Angular 17+). Also,/types/static/tokenizers.d.tsis not being generated.* Replaces internal path aliases (@utils,@core/*,@static/*) with relative imports so the emitted declarations are correct without any post-processing. This makestsc-aliaslonger needed and is removed.src/static/tokenizer.d.tstosrc/static/tokenizer.tsto ensuretscemitstypes/static/tokenizer.d.tstsconfig.jsonfromsrc/utils/index.tstosrc/utils/indexto ensuretsc-aliasrewrites@utilscorrectly.package.jsonWithout these changes, Angular 17+ (and other bundlers that resolve the "bundler" exports condition) fail to build with errors like: