fix: `systemjs` re-traverses helpers #16225

liuxingbaoyu · 2024-01-18T11:32:10Z

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 · 2024-01-18T11:34:11Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56174/

nicolo-ribaudo · 2024-01-18T11:34:55Z

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 exec.js test like the one described in the issue?

liuxingbaoyu · 2024-01-19T00:16:08Z

I tried, but we can't use exec.js with systemjs ~~and I haven't figured out a good way to test this.~~

liuxingbaoyu · 2024-01-19T03:21:39Z

CI is weird and fails randomly.
But I can't reproduce it locally.
May be related to zloirock/core-js#1292.

nicolo-ribaudo

I'm debugging this locally

nicolo-ribaudo · 2024-01-19T09:57:14Z

packages/babel-plugin-transform-modules-systemjs/test/index.js

+    const System = {
+      register: function (_, module) {
+        ret = module().execute();
+      },
+    };
+
+    eval((require, System, content));


I don't understand how this line works -- why do we need to use (require, System, content)?

But also, please do not use eval since the evaluated code can mess up with the global environment 😅 Could you use runCodeInTestContext instead?

Ok let's see if this makes the flakyness go away

This is just to make eslint happy. :)
I tried using vm, but it was blocked by require, and I didn't think of runCodeInTestContext.

liuxingbaoyu · 2024-01-20T00:22:07Z

@nicolo-ribaudo Thank you!

liuxingbaoyu · 2024-01-20T00:22:51Z

...es/babel-plugin-transform-modules-systemjs/test/fixtures/preset-env/requeue-helpers/input.js

+// delete global.Symbol doesn't work with jest in node 6
+Object.defineProperty(global, "Symbol", {
+  configurable: true,
+  writable: true,
+  value: undefined
+});


To be honest, I don't understand how jest affects our isolation environment in node 6.

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. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/random-bunny/pulls/142 Reviewed-by: Vylpes <ethan@vylpes.com> Co-authored-by: Renovate Bot <renovate@vylpes.com> Co-committed-by: Renovate Bot <renovate@vylpes.com>

liuxingbaoyu force-pushed the fix-typeof-systemjs branch from f070605 to c5c5424 Compare January 19, 2024 00:49

nicolo-ribaudo reviewed Jan 19, 2024

View reviewed changes

nicolo-ribaudo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Jan 19, 2024

liuxingbaoyu and others added 10 commits January 19, 2024 17:44

fix

8e6308f

review

37bebff

update tests

8fb57e3

avoid globalThis

5143912

test

7bb07d3

test

07b4e19

Run test in vm context

9cdf541

Ensure that the test is testing the correct behavior

d999bff

yarn dedupe

80960da

Regenerate makefile for deps update

a1765b2

nicolo-ribaudo force-pushed the fix-typeof-systemjs branch from e9c293e to a1765b2 Compare January 19, 2024 16:45

fix node 6

73cdf21

liuxingbaoyu force-pushed the fix-typeof-systemjs branch from 08c4110 to 73cdf21 Compare January 20, 2024 00:02

liuxingbaoyu commented Jan 20, 2024

View reviewed changes

nicolo-ribaudo approved these changes Jan 22, 2024

View reviewed changes

JLHwung approved these changes Jan 22, 2024

View reviewed changes

nicolo-ribaudo merged commit dfa4cc6 into babel:main Jan 22, 2024
50 checks passed

Frenzie mentioned this pull request Jan 26, 2024

Android WebView: Cannot convert a Symbol value to a string silvermine/videojs-airplay#53

Open

liuxingbaoyu added a commit to liuxingbaoyu/babel that referenced this pull request Mar 5, 2024

fix: systemjs re-traverses helpers (babel#16225)

ce8590d

Co-authored-by: Nicolò Ribaudo <hello@nicr.dev>

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Apr 23, 2024

github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: `systemjs` re-traverses helpers #16225

fix: `systemjs` re-traverses helpers #16225

liuxingbaoyu commented Jan 18, 2024

babel-bot commented Jan 18, 2024 •

edited

nicolo-ribaudo commented Jan 18, 2024

liuxingbaoyu commented Jan 19, 2024 •

edited

liuxingbaoyu commented Jan 19, 2024 •

edited

nicolo-ribaudo left a comment

nicolo-ribaudo Jan 19, 2024

nicolo-ribaudo Jan 19, 2024

liuxingbaoyu Jan 19, 2024

liuxingbaoyu commented Jan 20, 2024

liuxingbaoyu Jan 20, 2024

nicolo-ribaudo Jan 22, 2024

fix: systemjs re-traverses helpers #16225

fix: systemjs re-traverses helpers #16225

Conversation

liuxingbaoyu commented Jan 18, 2024

babel-bot commented Jan 18, 2024 • edited

nicolo-ribaudo commented Jan 18, 2024

liuxingbaoyu commented Jan 19, 2024 • edited

liuxingbaoyu commented Jan 19, 2024 • edited

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

nicolo-ribaudo Jan 19, 2024

Choose a reason for hiding this comment

nicolo-ribaudo Jan 19, 2024

Choose a reason for hiding this comment

liuxingbaoyu Jan 19, 2024

Choose a reason for hiding this comment

liuxingbaoyu commented Jan 20, 2024

liuxingbaoyu Jan 20, 2024

Choose a reason for hiding this comment

nicolo-ribaudo Jan 22, 2024

Choose a reason for hiding this comment

fix: `systemjs` re-traverses helpers #16225

fix: `systemjs` re-traverses helpers #16225

babel-bot commented Jan 18, 2024 •

edited

liuxingbaoyu commented Jan 19, 2024 •

edited

liuxingbaoyu commented Jan 19, 2024 •

edited