refactor: use neoqs and vitest #268
Conversation
| @@ -1,9 +1,9 @@ | |||
| import qs, { IStringifyOptions } from 'qs'; | |||
| import { stringify, StringifyOptions } from 'neoqs'; | |||
There was a problem hiding this comment.
Should be neoqs/legacy, as neoqs doesn't support CJS, but legacy subpath does
There was a problem hiding this comment.
why would that matter? my understanding is it's getting built into both esm and cjs by tsup from esm source
There was a problem hiding this comment.
Im not sure if current setup builds qs also in, based on my experience it prolly doesn't.
If it builds it in then its good, otherwise it should be switched to /legacy
There was a problem hiding this comment.
yes, I wish CI would trigger, but locally all tests pass
There was a problem hiding this comment.
tests won't catch this issue. Can you locally build and inspect whether the output file is importing qs or is it fully inlined?
There was a problem hiding this comment.
Good catch! Added noExternal from: https://www.jsdocs.io/package/tsup#Options
/** Always bundle modules matching given patterns */
noExternal?: (string | RegExp)[];
Summary
Fixes #267
Details
I couldn't get jest to work, so I converted to
vitestwhich works for me, but I can revert if that unacceptable