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

[test] remove dependency on esm #28617

Closed
11 tasks
jeremymeng opened this issue Feb 20, 2024 · 3 comments
Closed
11 tasks

[test] remove dependency on esm #28617

jeremymeng opened this issue Feb 20, 2024 · 3 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. test-reliability Issue that causes tests to be unreliable
Milestone

Comments

@jeremymeng
Copy link
Member

The majority of tests have been switched off esm in PR #28589. There are still a few packages that need more attention before they can remove usage of esm. Mainly due to the __dirname usage that is no available in ESM. This issue tracks fixing them.

Success Criteria:

@jeremymeng jeremymeng added Client This issue points to a problem in the data-plane of the library. test-reliability Issue that causes tests to be unreliable labels Feb 20, 2024
@jeremymeng
Copy link
Member Author

for *.js there's a fix of const __dirname = path.dirname(fileURLToPath(import.meta.url)); but unfortunately it wouldn't work for our test:node-ts-input because we set TS_NODE_COMPILER_OPTIONS to use "module": "commonjs", while using import.meta` requires "module" of ES2020, etc.

@maorleger maorleger self-assigned this Mar 8, 2024
maorleger added a commit that referenced this issue Mar 12, 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

****NO_CI****
@maorleger
Copy link
Member

maorleger commented Mar 12, 2024

Tasks remaining:

@maorleger maorleger added this to the 2024-05 milestone Mar 27, 2024
maorleger added a commit that referenced this issue Apr 15, 2024
We no longer use esm4mocha, instead relying on tsx for our node-js-input
tests.

Now that this has been in place for some time, we can remove esm4mocha
followed by the esm dependency.

Contributes to #28617
maorleger added a commit that referenced this issue Apr 17, 2024
### Packages impacted by this PR

All via dev-tool

### Issues associated with this PR

Contributes to #28617

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

esm is still being required in our test:node-ts-input and I don't
believe it is necessary. I'm removing it, but PLEASE do let me know what I'm missing -
I don't know much about mocha config to be honest.
@maorleger
Copy link
Member

Closed via 446e9d8 🎉

@github-actions github-actions bot locked and limited conversation to collaborators Jul 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. test-reliability Issue that causes tests to be unreliable
Projects
None yet
Development

No branches or pull requests

3 participants
@maorleger @jeremymeng and others