feat(deps): upgrade rollup, commonjs plugin #5274
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
| Path | Error Count |
|---|---|
| src/dev-server/index.ts | 37 |
| src/mock-doc/serialize-node.ts | 36 |
| src/dev-server/server-process.ts | 32 |
| src/compiler/prerender/prerender-main.ts | 22 |
| src/testing/puppeteer/puppeteer-element.ts | 22 |
| src/runtime/client-hydrate.ts | 20 |
| src/screenshot/connector-base.ts | 19 |
| src/runtime/vdom/vdom-render.ts | 17 |
| src/compiler/config/test/validate-paths.spec.ts | 16 |
| src/dev-server/request-handler.ts | 15 |
| src/compiler/prerender/prerender-optimize.ts | 14 |
| src/compiler/sys/stencil-sys.ts | 14 |
| src/compiler/transpile/transpile-module.ts | 14 |
| src/sys/node/node-sys.ts | 14 |
| src/compiler/prerender/prerender-queue.ts | 13 |
| src/compiler/sys/in-memory-fs.ts | 13 |
| src/runtime/connected-callback.ts | 13 |
| src/runtime/set-value.ts | 13 |
| src/compiler/output-targets/output-www.ts | 12 |
| src/compiler/transformers/test/parse-vdom.spec.ts | 12 |
Our most common errors
| Typescript Error Code | Count |
|---|---|
| TS2345 | 366 |
| TS2322 | 364 |
| TS18048 | 223 |
| TS18047 | 99 |
| TS2722 | 37 |
| TS2532 | 26 |
| TS2531 | 23 |
| TS2454 | 14 |
| TS2352 | 12 |
| TS2790 | 11 |
| TS2769 | 8 |
| TS2538 | 8 |
| TS2344 | 6 |
| TS2416 | 6 |
| TS2493 | 3 |
| TS18046 | 2 |
| TS2684 | 1 |
| TS2430 | 1 |
Unused exports report
There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!
Unused exports
| File | Line | Identifier |
|---|---|---|
| src/runtime/bootstrap-lazy.ts | 21 | setNonce |
| src/screenshot/screenshot-fs.ts | 18 | readScreenshotData |
| src/testing/testing-utils.ts | 198 | withSilentWarn |
| src/utils/index.ts | 145 | CUSTOM |
| src/utils/index.ts | 269 | normalize |
| src/utils/index.ts | 7 | escapeRegExpSpecialCharacters |
| src/compiler/app-core/app-data.ts | 25 | BUILD |
| src/compiler/app-core/app-data.ts | 115 | Env |
| src/compiler/app-core/app-data.ts | 117 | NAMESPACE |
| src/compiler/fs-watch/fs-watch-rebuild.ts | 123 | updateCacheFromRebuild |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | satisfies |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | Record |
| src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
| src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
alicewriteswrongs
approved these changes
Jan 18, 2024
christian-bromann
approved these changes
Jan 18, 2024
upgrade rollup to v2.56.3 and `@rollup/plugin-commonjs` to v21.1. this commit raises these libraries to the highest version as possible without introducing breaking changes. v2.57 has a bug in it that requires `@rollup/plugin-commonjs@22` for handling CJS-ESM interop. that version of `@rollup/plugin-commonjs` requires `@rollup/plugin-node-resolve@13`. however, `@rollup/plugin-node-resolve@11` has changes that would be breaking for existing users (see #5269) in these various versions of both libraries, new flags have been added to configure its output. for now, we continue to use the existing configurations to prevent breaking users. these new configurations have been recorded and tackled when we upgrade to newer versions of rollup (as a part of stencil v5). future upgrades will be slated to stencil v5.0.0. STENCIL-595
rwaskiewicz
force-pushed
the
rwaskiewicz/rollup-v2-minor-upgrade
branch
from
61a6230
to
1baacd7
Compare
|
This patch has been released as a part of today's Stencil v4.11.0 release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE! I want to squash this as a
feat()to surface the changes in the changelog. While I've tested this pretty well (subjective, I know 馃槈), I want folks to see this core piece of infra is being updated, as it impacts them as well.What is the current behavior?
Our rollup infrastructure is lagging behind the latest. We're currently working on a few different modernization efforts for handling newer JS paradigms + infra; this is one of them.
GitHub Issue Number: N/A
What is the new behavior?
upgrade rollup to v2.56.3 and
@rollup/plugin-commonjsto v21.1.this commit raises these libraries to the highest version as possible
without introducing breaking changes. v2.57 has a bug in it that
requires
@rollup/plugin-commonjs@22for handling CJS-ESM interop.that version of
@rollup/plugin-commonjsrequires@rollup/plugin-node-resolve@13. however,@rollup/plugin-node-resolve@11has changes that would be breakingfor existing users (see #5269)
future upgrades will be slated to stencil v5.0.0.
Does this introduce a breaking change?
Testing
I created a dev build of Stencil,
4.10.0-dev.1705594858.56d0fbaand used that in a CI run of Framework, which passed https://github.com/ionic-team/ionic-framework/actions/runs/7573347378I did a pretty comprehensive diff of the changes that are going to be applied to the Ionic Framework as a part of this. For every minor version of rollup of
@rollup/plugin-commonjs, I:npm run clean && npm ci && npm run build && npm pack)npm run build)components,css,dist,hydrate,loaderdirectories of the latest version I just built with the previousIn doing so, I noticed minimal changes. Happy to discuss them in a jam session or otherwise.
Other information
My notes for this are an attachment on the JIRA ticket.