deps: upgrade @rollup/plugin-commonjs to v22 to fix try/catch requires

[agilgur5]

@ezolenko this is a bit important as 0.32.0 is currently broken for some users. Ironically, I suggested calling out dependency changes in #332 (comment) and this issue is in fact due to a breaking change in a dependency. In a dev dependency at that!

Summary

Upgrade @rollup/plugin-commonjs to v22.0.0 to fix a breaking change in v21.0.0 that broke the bundling of graphlib and how it imports lodash, breaking rpt2 as a whole.

• Fixes ReferenceError: window is not defined in 0.32.0 #339

Details

• v21 broke graphlib's internal usage of lodash with its breaking change, and v22 has another breaking change to fix that

  • notably, this was breaking rpt2 imports for some users because rpt2 bundles in graphlib etc, meaning the nuances of the bundling got leaked to consumers, and, in fact, broke their builds
    • another upside to unbundling, i.e. Do not bundle dependencies #80, is that this situation wouldn't happen

• run a build as well since that's necessary for/vital to this change

Can see more details in my root cause analysis in #339 (comment) and the comments leading up to that. rollup/plugins#1005 (comment) is the central bug that applies to this case too.

Review Notes

Note how massive a change it is to the dist due to just changing the version of @rollup/plugin-commonjs. I can't even link to the line in the diff where graphlib's usage of lodash changed because GitHub won't load a diff that big. I can confirm though that it does change that same area of code that was broken, and that testing this branch/PR in a user's repo resolved the issue.

I was able to repro in a user's library but couldn't repro this in rpt2 itself (when it bundles itself as in) or in one of my libraries though, so it's possibly an environment dependent bug or a race condition.

This will need a release once merged.

[ezolenko]

Looks good (once you ignore all the whitespace :D). Do you think this should be 32.1 or 33.0? @agilgur5

[quisido]

This will need a release once merged.

I'm not sure the release process for this repository. Should this commit include a patch version bump?

[agilgur5]

once you ignore all the whitespace :D

Ah that's a good tip, I did not realize how much was whitespace changes

Do you think this should be 32.1 or 33.0? @agilgur5

@ezolenko 0.32.1 in my opinion, since 0.32.0 is partly broken due to this -- like literally unusable in certain environments. So can just consider this a "follow-up" release in that sense, since 0.32.0 wasn't entirely usable anyway.
In my opinion, I believe this to be a bugfix rather than a breaking change. I would also recommend deprecating 0.32.0 on NPM (in a week or so for less confusion) with a message saying to upgrade to 0.32.1.
This change should semantically make the dependencies work as they did in 0.31, and I think that's the main breakage in 0.32.0.
(The rest of the main changes are in 0.32.0 are "bugfixes that change some incorrect semantics"; could consider all of them non-breaking as well IMO).

Also, just for reference, I try to point out things one may consider as breaking in PRs too.

I'm not sure the release process for this repository. Should this commit include a patch version bump?

@CharlesStover generally there's a separate commit that ezolenko makes that only bumps versions.
That's done in most repos I've worked with, especially since a release may batch a few PRs together.

[ezolenko]

Yep, release process goes roughly like this:

• package version is set at least a patch version higher than previous release (usually this is done right after previous release)
• if the changes look breaking, a minor version is incremented
• I do a build, review the diff and publish on npm
• build is then committed to git (if it wasn't already) and git release archive is made from that revision (so what's in npm and what's in git release under the same tag should match exactly)
• package version is then incremented

[ezolenko]

That second point (about minor version bump) is a pure judgement call. Hopefully it would be more substantiated eventually if we get integrations tests and stuff. :)

[ezolenko]

This is released in 0.32.1 btw

[agilgur5]

Yep, release process goes roughly like this:

It might be good to add your exact steps, specifically the commands you run, into a "Publishing" section (under "Development") in the new CONTRIBUTING.md.
So that way it's explicitly described and other people can replicate it (and in the future maybe, so that others can release too). Also from personal experience, I legit sometimes forget my own publishing process if I haven't released in a bit 😅

That second point (about minor version bump) is a pure judgement call. Hopefully it would be more substantiated eventually if we get integrations tests and stuff. :)

💯 integration tests will probably be the next big thing I tackle as they're really starting to be necessary to make sure that some of the deeper bugfixes I'm making don't break a bunch of things.

But even then, version bumps are always subjective to an extent.
Even when I've tried to reason about a change with ComVer (which has no bugfixes), there are still some cases where something could be considered breaking, but was intended as more of a feature.
Or, in a broader sense, what one identifies as "the API surface" is itself subjective. For instance, a hypothetical: are thrown errors an API surface? In which case, changing an error message to be "more helpful" (i.e. a "feature") could also be considered breaking. 🤯 🤯 (actual scenario from one of my libraries: agilgur5/react-signature-canvas#42 (comment))