Skip to content
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

remove use-esm-workaround #28826

Merged
merged 4 commits into from
Mar 12, 2024
Merged

Conversation

maorleger
Copy link
Member

@maorleger maorleger commented Mar 7, 2024

Builds off of Matt's work on moving to tsx in #28801 by removing the use-esm-workaround flag from packages
that needed it before we moved to tsx.

There's additional cleanup to be had, but I am trying not to cause a build storm.

We are at a point where we can delete esm globally!

Contributes to #28617 which can be completed with a no-ci change to remove esm globally

  • Add NO_CI message for main

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@maorleger maorleger changed the title remove use esm workaround remove use-esm-workaround Mar 8, 2024
@xirzec
Copy link
Member

xirzec commented Mar 11, 2024

@maorleger can you merge main?

@maorleger maorleger force-pushed the remove-use-esm-workaround branch from 60a10ae to 49d0af3 Compare March 12, 2024 15:11
@maorleger maorleger requested review from srnagar, lmolkova, a team and hectorhdzg as code owners March 12, 2024 15:11
Copy link
Member

@xirzec xirzec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Assuming this pleases CI, I think we should be good to go.

@xirzec
Copy link
Member

xirzec commented Mar 12, 2024

/azp run js - ai-document-intelligence-rest - tests

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maorleger
Copy link
Member Author

maorleger commented Mar 12, 2024

min/max is failing because it does not use dev-tool commands today https://github.com/maorleger/azure-sdk-for-js/blob/main/eng/tools/dependency-testing/templates/package.json#L12

We should update that as well, but in a separate PR given the number of checks already run on this PR (opened #28890)

@xirzec
Copy link
Member

xirzec commented Mar 12, 2024

/check-enforcer override

@maorleger
Copy link
Member Author

maorleger commented Mar 12, 2024

/azp run js - identity - tests

Edit: want to see if identity nightly tests are fixed by this as well..

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

maorleger added a commit that referenced this pull request Mar 12, 2024
### Packages impacted by this PR

N/A

### Issues associated with this PR

Build failures

### Describe the problem that is addressed by this PR

`esm` is slowly becoming a larger problem for us and we've been on a
mission to remove it. min/max tests fail because esm is unable to handle
safe-navigation operators (?.) 

While a separate PR switches tests to use tsx, this PR focuses on
min/max tests and can be merged separately.

### Provide a list of related PRs _(if any)_

#28826
@maorleger
Copy link
Member Author

/check-enforcer override

@maorleger maorleger merged commit e3aaa5a into Azure:main Mar 12, 2024
151 of 154 checks passed
@maorleger maorleger deleted the remove-use-esm-workaround branch March 12, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants