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.

Sign up for GitHub

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

Jump to bottom

[Bug]: babel compile proposal-explicit-resource-management different from tsc #16435

Closed
1 task
loynoir opened this issue Apr 18, 2024 · 6 comments · Fixed by #16440
Closed
1 task

[Bug]: babel compile proposal-explicit-resource-management different from tsc #16435

loynoir opened this issue Apr 18, 2024 · 6 comments · Fixed by #16440
Assignees
@sossost
Labels
good first issue i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue Spec: Explicit Resource Management

Comments

@loynoir
Copy link

loynoir commented Apr 18, 2024 •
edited
Loading

💻

  • Would you like to work on a fix?

How are you using Babel?

@babel/cli

Input code

microsoft/TypeScript#58234

import { deepStrictEqual } from 'node:assert'
import process from 'node:process'

let i = 0
/** @type {any} */
let err
try {
  await using _x1 = {
    async [Symbol.asyncDispose]() {
      throw [1,   i]
    }
  }

  await using _x2 = {
    async [Symbol.asyncDispose]() {
      throw [2,   i]
    }
  }

  await using _x3 = {
    async [Symbol.asyncDispose]() {
      throw [3,   i]
    }
  }

  await using _x4 = {
    async [Symbol.asyncDispose]() {
      throw [4,   i]
    }
  }

  throw [5,   i]
} catch (e) {
  err = e
}

switch (process.env['TRANSPILER']) {
  case 'tsc':
    deepStrictEqual(err.error, [1, 5])
    deepStrictEqual(err.suppressed.error, [2, 4])
    deepStrictEqual(err.suppressed.suppressed.error, [3, 3])
    deepStrictEqual(err.suppressed.suppressed.suppressed.error, [4, 2])
    deepStrictEqual(err.suppressed.suppressed.suppressed.suppressed, [5, 1])
    break
  case 'swc':
    deepStrictEqual(err.suppressed, [1, 5])
    deepStrictEqual(err.error.suppressed, [2, 4])
    deepStrictEqual(err.error.error.suppressed, [3, 3])
    deepStrictEqual(err.error.error.error.suppressed, [4, 2])
    deepStrictEqual(err.error.error.error.error, [5, 1])
    break
  case 'babel':
    deepStrictEqual(err.suppressed, [1, 5])
    deepStrictEqual(err.error.suppressed, [2, 4])
    deepStrictEqual(err.error.error.suppressed, [3, 3])
    deepStrictEqual(err.error.error.error.suppressed, [4, 2])
    deepStrictEqual(err.error.error.error.error, [5, 1])
    break
  default:
    // break
    throw new Error()
}

// export { err }

Configuration file name

No response

Configuration

No response

Current and expected behavior

Currently, tsc and swc and babel does not have same runtime behavior.

Expect tsc and swc and babel have same runtime behavior.

Environment

7.24.1 (@babel/core 7.24.4)

Possible solution

No response

Additional context

No response
@babel-bot
Copy link
Collaborator

Hey @loynoir! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

@nicolo-ribaudo
Copy link
Member

If anybody wants to fix this, the problem is in the order we pass errors to the SuppressedError constructor in https://github.com/babel/babel/blob/33d8d7d54e1d2eb2d55c455a3ff34a73b55bd569/packages/babel-helpers/src/helpers/usingCtx.ts. After fixing that, we probably need to update some tests that are asserting the wrong thing.

This would be a good first contribution for anybody interested :)

If it is the first time that you contribute to Babel, follow these steps: (you need to have make and yarn available on your machine)

  1. Write a comment there to let other possible contributors know that you are working on this bug.
  2. Fork the repo
  3. Run git clone https://github.com/<YOUR_USERNAME>/babel.git && cd babel
  4. Run yarn && make bootstrap
  5. Wait ⏳
  6. Run make watch (or make build whenever you change a file)
    • NOTE: When changing helpers code, make watch is not always reliable so you should run make build
  7. Add a test (only input.js; output.js will be automatically generated)
  8. Update the code!
  9. yarn jest explicit-resource-management to run the tests
    • You will probably need to update some assertions in failing tests
    • If some snapshot test outputs don't match but the new results are correct, you can delete the bad output.js files and run the tests again
      • If you prefer, you can run OVERWRITE=true yarn jest explicit-resource-management and they will be automatically updated.
  10. If it is working, run make test to run all the tests
  11. Run git push and open a PR!

@sossost
Copy link
Contributor

sossost commented Apr 19, 2024

hello! @nicolo-ribaudo I am sossost, a new contributor. I would like to solve this problem. Is it possible?

@nicolo-ribaudo
Copy link
Member

Yes! Feel free to ask here if you need any help :)

@sossost
Copy link
Contributor

sossost commented Apr 19, 2024

thank you! If there is anything I need help with along the way, I will ask. have a good day

@sossost sossost mentioned this issue Apr 20, 2024
Merged
@sossost
Copy link
Contributor

sossost commented Apr 20, 2024

@nicolo-ribaudo Thanks for your help. as you mentioned, I modified the order of sending errors to SuppressedError and modified related tests.
Am I doing it right?

@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jul 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@sossost sossost

Labels
good first issue i: bug outdated A closed issue/PR that is archived due to age. Recommended to make a new issue Spec: Explicit Resource Management
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix suppressed error order
5 participants
@nicolo-ribaudo @babel-bot @liuxingbaoyu @loynoir @sossost