Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.7k
fix:systemjs re-traverses helpers#16225
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
liuxingbaoyu commentedJan 18, 2024
| Q | A |
|---|---|
| Fixed Issues? | Fixes#16219 |
| Patch: Bug Fix? | √ |
| Major: Breaking Change? | |
| Minor: New Feature? | |
| Tests Added + Pass? | √ |
| Documentation PR Link | |
| Any Dependency Changes? | |
| License | MIT |
babel-bot commentedJan 18, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Build successful! You can test your changes in the REPL here:https://babeljs.io/repl/build/56174/ |
Given that the inline helper is difficult to read and if we regress it's likely we would just think "oh a change in the helper, lets commit it", could you add an |
liuxingbaoyu commentedJan 19, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I tried, but we can't use |
f070605 toc5c5424Compareliuxingbaoyu commentedJan 19, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CI is weird and fails randomly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm debugging this locally
| const System = { | ||
| register: function (_, module) { | ||
| ret = module().execute(); | ||
| }, | ||
| }; | ||
| eval((require, System, content)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't understand how this line works -- why do we need to use(require, System, content)?
But also, please do not useeval since the evaluated code can mess up with the global environment 😅 Could you userunCodeInTestContext instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok let's see if this makes the flakyness go away
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is just to make eslint happy. :)
I tried usingvm, but it was blocked byrequire, and I didn't think ofrunCodeInTestContext.
e9c293e toa1765b2Compare08c4110 to73cdf21Compare@nicolo-ribaudo Thank you! |
| // delete global.Symbol doesn't work with jest in node 6 | ||
| Object.defineProperty(global, "Symbol", { | ||
| configurable: true, | ||
| writable: true, | ||
| value: undefined | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
To be honest, I don't understand how jest affects our isolation environment in node 6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Jest does bad things to the global object to implement isolation-but-not-too-much, this workaround is good enough.
Co-authored-by: Nicolò Ribaudo <hello@nicr.dev>
This PR contains the following updates:| Package | Type | Update | Change ||---|---|---|---|| [@babel/traverse](https://babel.dev/docs/en/next/babel-traverse) ([source](https://github.com/babel/babel)) | resolutions | patch | [`7.23.7` -> `7.23.9`](https://renovatebot.com/diffs/npm/@babel%2ftraverse/7.23.7/7.23.9) |---### Release Notes<details><summary>babel/babel (@​babel/traverse)</summary>### [`v7.23.9`](https://github.com/babel/babel/blob/HEAD/CHANGELOG.md#v7239-2024-01-25)[Compare Source](babel/babel@v7.23.7...v7.23.9)##### 🐛 Bug Fix- `babel-helper-transform-fixture-test-runner`, `babel-plugin-transform-function-name`, `babel-plugin-transform-modules-systemjs`, `babel-preset-env` - [#​16225](babel/babel#16225) fix: `systemjs` re-traverses helpers ([@​liuxingbaoyu](https://github.com/liuxingbaoyu))- `babel-helper-create-class-features-plugin`, `babel-plugin-proposal-decorators` - [#​16226](babel/babel#16226) Improve decorated private method check ([@​JLHwung](https://github.com/JLHwung))- `babel-plugin-proposal-decorators`, `babel-plugin-transform-async-generator-functions`, `babel-plugin-transform-runtime`, `babel-preset-env` - [#​16224](babel/babel#16224) Properly sort `core-js@3` imports ([@​nicolo-ribaudo](https://github.com/nicolo-ribaudo))- `babel-traverse` - [#​15383](babel/babel#15383) fix: Don't throw in `getTypeAnnotation` when using TS+inference ([@​liuxingbaoyu](https://github.com/liuxingbaoyu))- Other - [#​16210](babel/babel#16210) \[eslint] Fix `no-use-before-define` for class ref in fields ([@​nicolo-ribaudo](https://github.com/nicolo-ribaudo))##### 🏠 Internal- `babel-core`, `babel-parser`, `babel-template` - [#​16222](babel/babel#16222) Migrate `eslint-parser` to cts ([@​liuxingbaoyu](https://github.com/liuxingbaoyu))- `babel-types` - [#​16213](babel/babel#16213) Remove `@babel/types` props that are not produced by the parser ([@​liuxingbaoyu](https://github.com/liuxingbaoyu))##### 🏃♀️ Performance- `babel-parser` - [#​16072](babel/babel#16072) perf: Improve parser performance for typescript ([@​liuxingbaoyu](https://github.com/liuxingbaoyu))##### 🔬 Output optimization- `babel-helper-create-class-features-plugin`, `babel-plugin-proposal-decorators`, `babel-plugin-proposal-destructuring-private`, `babel-plugin-proposal-pipeline-operator`, `babel-plugin-transform-class-properties`, `babel-plugin-transform-class-static-block`, `babel-plugin-transform-new-target`, `babel-plugin-transform-parameters`, `babel-plugin-transform-private-methods`, `babel-preset-env` - [#​16218](babel/babel#16218) Improve temporary variables for decorators ([@​liuxingbaoyu](https://github.com/liuxingbaoyu))- `babel-helpers`, `babel-plugin-proposal-explicit-resource-management`, `babel-runtime-corejs2`, `babel-runtime-corejs3`, `babel-runtime` - [#​15959](babel/babel#15959) Improve output of `using` ([@​liuxingbaoyu](https://github.com/liuxingbaoyu))</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this update again.--- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box---This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjAiLCJ0YXJnZXRCcmFuY2giOiJkZXZlbG9wIn0=-->Reviewed-on:https://gitea.vylpes.xyz/RabbitLabs/random-bunny/pulls/142Reviewed-by: Vylpes <ethan@vylpes.com>Co-authored-by: Renovate Bot <renovate@vylpes.com>Co-committed-by: Renovate Bot <renovate@vylpes.com>