chore: upgrade dependencies

mcmxcdev commented 2 years ago

Description

The unit test are still passing and rollup is building fine as before. ~~I wasn't able to test it locally, I guess I would need to pnpm link it?~~

You might want to have a look at the warnings reported by rollup (that were there before too) and solve or suppress them. additionally, I would recommend specifying the minimum required node version in engines to show expected compatibility.

I was able to upgrade all dependencies except for p-queue, which needs to be transpiled to be able to be tested in jest since it is ESM with the new major version.

Changes

add pnpm engines config to package.json
update pnpm-lock.yaml lockfileVersion with pnpm 7
upgrade jest and ts-jest to v28
upgrade node-html-parser and string-replace-async to next major version
upgrade various dependencies to next patch/minor version
install missing peer dependencies

Open issues

When linking this PR locally to a project that uses svimg, this is the output:

> svelte-kit dev

> require() of ES Module /home/<user>/svimg/node_modules/.pnpm/string-replace-async@3.0.2/node_modules/string-replace-async/index.js from /home/<user>/svimg/dist/index.js not supported.
Instead change the require of /home/<user>/svimg/node_modules/.pnpm/string-replace-async@3.0.2/node_modules/string-replace-async/index.js in /home/<user>/svimg/dist/index.js to a dynamic import() which is available in all CommonJS modules.

So it would be necessary to convert the project to be ESM friendly.

xiphux commented 2 years ago

Thanks. Looks like the GitHub Actions probably need to be updated for pnpm 7 for the new lockfile (it currently pins to 6.x.x).

Yeah, I think the reason I hadn't updated string-replace-async was because it was ESM. I wasn't quite prepared to convert svimg to ESM yet because it uses Typescript and I believe typescript support for Node ESM is still experimental.

I'll probably need to do some testing with the newer node-html-parser - I ran into some odd issues with it before in #14 under certain circumstances (non-link development or using npm instead of pnpm) which is why I had pinned the version to one I knew was working.

mcmxcdev commented 2 years ago

Okay, how do you suggest to proceed?

We could keep this PR open until you want to convert to ESM, or I undo the major version changes (except for Jest) to be able to merge it.

xiphux commented 2 years ago

It's up to you, I don't mind doing any of these updates that don't require an ESM conversion, which I think would be everything except the major version breaks of node-html-parser 5.x.x and string-replace-async 3.x.x.

I'd still want to run some manual tests on the PR if we unpin node-html-parser to the latest 4.x.x (4.1.5 looks like) to avoid any regressions, but I don't mind doing that.

mcmxcdev commented 2 years ago

I pushed another commit that downgrades the packages with ESM version changes again.

Additionally, pnpm is now run with v7 in GitHub Actions. Note that I also updated the Node version matrix (e.g. Node v12 is EOL) and updated e.g. actions/setup-node to v3. If you stick to strict semver, this would require a v3 release of svimg.

xiphux / svimg