Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Style error information
https://eslint.org/docs/rules/no-restricted-globals
How to Review
## 1. Apply Patch
## 2. Review Code Changes
## 3. Confirm no Code Standard Errors
yarn & yarn lint:core-js-passing
## 4.1 If `NO` errors, mark the issue as `Reviewed & tested by the community` (Don't be shy, we're all friendly)
## 4.2 If `HAS` errors, fix it and upload a new patch (Just do it and you can!!!)
Background
- #2912962: Step 1 JS codestyle: [meta] Fix JS coding standards in core
- We adapted the airbnb coding standard (#2815077: Adopt airbnb javascript style guide v14.1 as new baseline javascript coding standards for Drupal 8 core and contrib), but we are not fully compliant to it yet.
More Information
- Using ES6 in Core
https://www.drupal.org/node/2815083
- To find JS code standard errors stats
cd core/ && yarn & yarn lint:core-js-stats
Valuable Follow-up
- N/A
Comment | File | Size | Author |
---|---|---|---|
#10 | 2983373-10.patch | 9.44 KB | GrandmaGlassesRopeMan |
#7 | interdiff-2983373-2-7.txt | 882 bytes | ApacheEx |
#7 | 2983373-7.patch | 9.43 KB | ApacheEx |
Comments
Comment #2
ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commentedHere is a patch. I have also run
yarn run build:js
.Comment #4
m1r1k CreditAttribution: m1r1k commentedSeems like babel does not provide polyfill? Because Number.isNan is ES6 feature and in disaplace.js it is not changed
Comment #5
m1r1k CreditAttribution: m1r1k commentedStyles were fixed, but what to do with Number.isNan polyfill?
How do you usually proceed guys? :)
1) manual polyfill
2) import 'core-js/es6/number';
3) etc
Comment #6
ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commentedthanks for your investigation. IE doesn't support
Number.isNaN
, so yeah perhaps we need to polyfill it.I will take a look today later.
Comment #7
ApacheEx CreditAttribution: ApacheEx as a volunteer and at Internetdevels, Drupal Ukraine Community commentedI was trying to use https://babeljs.io/docs/en/babel-plugin-transform-runtime,
it will add polyfills automatically only when needed (based on browsers from babel-present-env).
But, that's something which depends on
require
orimport
- we don't support it ¯\_(ツ)_/¯I see 2 ways:
1) ignore it by
eslint-disable-next-line
2) add https://polyfill.io/ or properly configure https://babeljs.io/docs/en/babel-polyfill
currently, I'd prefer the first option. Any other thoughts?
Comment #8
m1r1k CreditAttribution: m1r1k commentedas @drpal mentioned somewhere in other tasks, it is quite costy to add polyfills in such cases, because they will be added to all separate files and we can handle duplication while we transpile each file separately
Comment #9
GrandmaGlassesRopeManNice. I think this fine for now.
@m1r1k & @ApacheEx - You are correct. Right now we don't have a solid plan for how we can handle polyfill support. If you want to open a followup where we can discuss an approach that would be wonderful.
Comment #10
GrandmaGlassesRopeMan- I forgot this minor reroll due to changes to
core/.eslintrc.passing.json
.Comment #11
ApacheEx CreditAttribution: ApacheEx as a volunteer and at Drupal Ukraine Community commentedThanks for reroll.
Here is a followup #2985495: JS: Polyfills support
Comment #14
lauriiiCommitted 258a27b and pushed to 8.7.x. Cherry-picked d1b8295 and pushed to 8.6.x. Thanks! ✨