Open Burnett2k opened 3 months ago
v9 is fine for docs / core spectacle.
Making mdx loader ESM is a bigger deal because we're cutting off CJS users.
@ryan-roemer In case the ESM / CommonJS issue comes up again, what is Formidable's stance? Sounds like we plan to support CJS users as long as possible?
If we released this upgrade as a major version upgrade, it wouldn't quite be cutting off users. But I do see how it could be perceived that way by the community.
It does seem if we want to continue to modernize the package with new features, new markdown syntax support, and keep it up-to-date, we will eventually need to upgrade or look for alternative dependencies which don't force ESM.
This all stems from some fairly low priority security issues and since we aren't pushing for new features within spectacle, I'm quite fine to just punt on this indefinitely until we find a more pressing need to upgrade.
I'm interested in @carbonrobot 's input here, but my thoughts are that we don't have a strict "stance" as much as want to follow what the communities of our various OSS can best be served by. For example:
Let's look at one example -- the popular chalk library from Sindre, who's an early ESM-only converter: https://www.npmjs.com/package/chalk?activeTab=versions (I'm ignoring small numbered versions)
The overwhelming usage is still CJS despite original 5.0.0 version published over two years ago.
For us here for spectacle, we could consider it a special case as Spectacle is consumed as a frontend library. spectacle-mdx-loader
is a Node library, but it's limited to just the build. So not a completely clear cut case, but I'd probably favor sticking with CJS rather than forcing all folks who want to create MDX decks to switch to ESM.
This is a complicated topic, so I will do my best to summarize all of the issues at play here. Primarily, this is on our radar because dependabot alerted us that sub dependencies reference a vulnerable package of trim. Unfortunately, there is not a clear or easy upgrade path to make this happen. Additionally, it's somewhat debatable whether or not this vulnerability should be considered as high priority as dependabot claims as it's a simple DoS regex. Since this is a presentation library, there's not much Denial of Service things you can do other than mess up your own decks which you can already do anyways ;)
Anyways, to get us off of trim completely, here's what I think we need to do. I might have missed a few things.
upgrade mxs-js
remark-parse@8.0.3
relevant docs:
output of pnpm why (excluding the dev-dependency docusaurus)