New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update dependencies #200
Update dependencies #200
Conversation
* Update to TS 4.9 and fix resulting issues * Add casts to prevent TS errors showing up when debugging apps that step into polyfill code * switch to @rollup/plugin-terser plugin * add version override for rollup-plugin-sourcemaps (verified in source that this seems OK) * Added JSON type assertion for importing package.json, because otherwise there were build errors. * Switched to using default imports for all rollup plugins
Seems that parts of the CI don't like the JSON assert. Maybe we should split the build and test actions in the CI config so that all the build steps use the same consistent Node env (17?) and then the test steps use whatever relevant version of Node (for most non-test actions, same as the version above, for test actions whatever version we are aiming to test against)? |
Instead of worrying about backwards-compatibility for Rollup, I think we should only ever run it on a recent Node version. #202 should fix the issues with Node14 tests without having to use any backward-compat stuff. It's weird that Rollup doesn't like our config because it's not CJS... |
I researched this. The issue (if I understand it correctly which I very well might not be!) is that some newer rollup plugins are ESM-only, and they seem to have trouble loading configs in pre-Node17 environments because in those environments, .js files are assumed to be CJS. I'm sure I'm mangling the explantation, but in any case there's a
@12wrigja whoops, I just saw these comments after I checked in workarounds for both issues. I'm open to your proposals for both of these. My only concern is a bit of chicken-and-egg where this PR introduces new dependencies and your PRs change how the build process works but don't validate against these new dependencies. Someone needs to validate that your build process changes work against these new deps. Given that this PR is currently all green, should we merge it (including the backwards-compat Node14 and CJS config workarounds) and then your build-process PRs can revert those workarounds assuming that they fix everything? Or should we merge those other PRs first and them remove the workarounds from this one? (Assuming they work with these new deps.) I'm OK either way, although I have a slight preference for merging this PR as-is because I'm lazy. :-) |
Feel free to merge this as-is and I'll play around with just how much is fixed by my other PR. |
Added JSON type assertion for importing package.json, because otherwise there were build errors.Instead of a type assertion (which failed in older Node versions), switched to using a downlevel-compatible way to load package.json. After we no longer need to support Node 14, we can switch back to using imports with type assertions.
--bundleConfigAsCjs
rollup CLI option. See [v3.0] Always try to load config files via Node if possible rollup/rollup#4621 for context.