[dev-tool] Upgrade to rollup v3 #27285
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
deyaaeldeen
force-pushed
the
rollup3-upgrade
branch
from
1ee9568 to
58841f0
Compare
|
API change check API changes are not detected in this pull request. |
jeremymeng
reviewed
Sep 29, 2023
jeremymeng
reviewed
Sep 29, 2023
| ); | ||
| } | ||
|
|
||
| function ignoreRheaPromiseCircularDependency(warning: RollupWarning): boolean { | ||
| return ( | ||
| warning.code === "CIRCULAR_DEPENDENCY" && | ||
| matchesPathSegments(warning.importer, ["node_modules", "rhea-promise"]) | ||
| matchesPathSegments(warning.ids?.[0], ["node_modules", "rhea-promise"]) |
There was a problem hiding this comment.
can you verify by with event-hubs or service-bus? When I tried, both id, or ids are undefined for rhea-promise warnings. I guess that was why we used importer. Even though Rollup v3 changed the type of RollupWarning, it looks that those warnings still have the same shape in JavaScript. I had to use message
Maybe also worth to log a rollup/rollup plugin bug if we expect valid ids.
There was a problem hiding this comment.
They build fine:
$ rushx build
Your version of Node.js (19.9.0) has not been tested with this release of Rush. Please consider installing a newer version of the "@microsoft/rush" package, or downgrading Node.js.
Found configuration in /home/codespace/window1/rush.json
Rush Multi-Project Build Tool 5.106.0 - Node.js 19.9.0 (unstable)
Your version of Node.js (19.9.0) is an odd-numbered release. These releases frequently have bugs. Please consider installing a Long Term Support (LTS) version instead.
> "npm run clean && tsc -p . && dev-tool run bundle && api-extractor run --local && npm run build:types"
> @azure/event-hubs@5.11.3 clean
> rimraf dist dist-* types *.tgz *.log
[bundle] Created production CommonJS bundle.
[bundle] Created browser testing bundle.
api-extractor 7.36.4 - https://api-extractor.com/
Using configuration from ./api-extractor.json
Analysis will use the bundled TypeScript version 5.0.4
API Extractor completed successfully
> @azure/event-hubs@5.11.3 build:types
> downlevel-dts types/latest types/3.1
There was a problem hiding this comment.
oh right, event-hubs and service-bus have rollup v2 because they have custom test config. build:test uses a different rollup.test.config.js which may also needs. could be another PR to upgrade them.
There was a problem hiding this comment.
Ah the libraries that polyfill node builtins are still on v2. Addressing them in a different PR sounds good.
jeremymeng
approved these changes
Sep 29, 2023
deyaaeldeen
requested review from
ckairen,
mikeharder,
minhanh-phan,
vicancy,
xirzec and
bterlson
as code owners
|
/checkenforcer override |
|
/check-enforcer evaluate |
This was referenced Oct 10, 2023
Merged
deyaaeldeen
added a commit
that referenced
this pull request
Oct 10, 2023
### Packages impacted by this PR @azure/synapse-monitoring ### Issues associated with this PR #27285 ### Describe the problem that is addressed by this PR Upgrading rollup to v3 and fixing compilation, bundling, and testing issues. ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? N/A ### Are there test cases added in this PR? _(If not, why?)_ N/A ### Provide a list of related PRs _(if any)_ N/A ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)
deyaaeldeen
added a commit
that referenced
this pull request
Oct 11, 2023
### Packages impacted by this PR [@azure/web-pubsub](https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3154068&view=results) [@azure/ai-form-recognizer](https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3154070&view=results) [@azure/communication-identity](https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3154080&view=results) [@azure/core-amqp](https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3154074&view=results) api management libs ### Issues associated with this PR #27285 ### Describe the problem that is addressed by this PR Upgrading rollup to v3 ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? N/A ### Are there test cases added in this PR? _(If not, why?)_ N/A ### Provide a list of related PRs _(if any)_ N/A ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)
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.
Packages impacted by this PR
All libraries
Issues associated with this PR
Fixes #26665
Describe the problem that is addressed by this PR
We need to upgrade Rollup to v3 so our cjs bundles could import ES modules among other goodness.
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
See the linked issue for discussion of the upgrade. Credit to @xirzec for writing up the sourcemaps plugin in-house replacement!
Are there test cases added in this PR? (If not, why?)
N/A
Provide a list of related PRs (if any)
N/A
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists