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

chore: drop structuredClone polyfill for v9 #17915

Merged
merged 1 commit into from Dec 29, 2023

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented Dec 29, 2023

I noticed this comment about a thing which could be removed and wanted to make sure it didn't get lost. Happy to close this if you don't want to worry about it.

This doesn't pass linting because no-undef claims that structuredClone is not defined, but I couldn't for the life of me figure out why. The globals package has listed structuredClone in node since 13.12.0. Fixed by upgrade to eslint-plugin-n, see below.

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

Removes a dependency which is no longer necessary in v9.

What changes did you make? (Give an overview)

Removed a polyfill.

Is there anything you'd like reviewers to focus on?

@eslint-github-bot eslint-github-bot bot added breaking This change is backwards-incompatible feature This change adds a new feature to ESLint labels Dec 29, 2023
Copy link

netlify bot commented Dec 29, 2023

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit b1a9424
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/658e43e974690e0008ee5107

Copy link

linux-foundation-easycla bot commented Dec 29, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: bakkot / name: Kevin Gibbons (b1a9424)

@mdjermanovic
Copy link
Member

This doesn't pass linting because no-undef claims that structuredClone is not defined, but I couldn't for the life of me figure out why. The globals package has listed structuredClone in node since 13.12.0.

@aladdin-add this seems to be related to eslint-plugin-n?

@aladdin-add
Copy link
Member

aladdin-add commented Dec 29, 2023

@aladdin-add this seems to be related to eslint-plugin-n?

Yes, will take a look later!

also, I'm changing the pr title "feat!" => "chore", as it's a not a user-facing change.

@aladdin-add aladdin-add changed the title feat!: drop structuredClone polyfill for v9 feat: drop structuredClone polyfill for v9 Dec 29, 2023
@aladdin-add aladdin-add added chore This change is not user-facing and removed breaking This change is backwards-incompatible feature This change adds a new feature to ESLint labels Dec 29, 2023
@aladdin-add aladdin-add changed the title feat: drop structuredClone polyfill for v9 chore: drop structuredClone polyfill for v9 Dec 29, 2023
@mdjermanovic
Copy link
Member

In the meantime, I think we can enable the global variable manually in this PR to make the linting pass.

eslint/eslint.config.js

Lines 100 to 102 in 8792464

languageOptions: {
ecmaVersion: "latest"
},

languageOptions: {
-    ecmaVersion: "latest",
+    ecmaVersion: "latest",
+    globals: {
+       structuredClone: "readonly"
+    }
},

@mdjermanovic
Copy link
Member

Okay, this works now with the new version of eslint-plugin-n, there's no need to update our eslint.config.js.

@mdjermanovic mdjermanovic marked this pull request as ready for review December 29, 2023 12:04
@mdjermanovic mdjermanovic requested a review from a team as a code owner December 29, 2023 12:04
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Would like @nzakas to verify before merging.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nzakas nzakas merged commit f6f4a45 into eslint:main Dec 29, 2023
33 of 35 checks passed
@bakkot bakkot deleted the drop-structured-clone-polyfill branch December 29, 2023 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore This change is not user-facing contributor pool
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

None yet

4 participants