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

Support numeric separators #13568

Closed
5 tasks done
mdjermanovic opened this issue Aug 15, 2020 · 12 comments
Closed
5 tasks done

Support numeric separators #13568

mdjermanovic opened this issue Aug 15, 2020 · 12 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features new syntax This issue is related to new syntax that has reached stage 4 rule Relates to ESLint's core rules

Comments

@mdjermanovic
Copy link
Member

mdjermanovic commented Aug 15, 2020

Numeric separators proposal is now in Stage 4.

https://github.com/tc39/proposal-numeric-separator

ESLint's default parser will support this syntax under ecmaVersion: 2021

@mdjermanovic mdjermanovic added rule Relates to ESLint's core rules core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion new syntax This issue is related to new syntax that has reached stage 4 labels Aug 15, 2020
@mdjermanovic
Copy link
Member Author

I believe there's nothing more that should be updated in ESLint core and ESLint core rules, aside from those 2 items listed above.

It would be nice to also add some tests for quote-props and prefer-numeric-literals.

@anikethsaha
Copy link
Member

anikethsaha commented Aug 16, 2020

I think for no-loss-of-precision only tests needs to be updated cause node.value will consist of non-numeric separated number and in no-loss-of-precision it is using node.value itself.

It needs to be updated, there are some places where it is using node.raw

@anikethsaha
Copy link
Member

I can work on no-loss-of-precision if it's ok.

@mdjermanovic
Copy link
Member Author

@anikethsaha

We usually prepare one big PR that covers all new features that are going to be supported by the next espree version, because all changes should be ideally merged and released at once, right after releasing espree.

I guess it would be easier to review and approve separate PRs, e.g., one per new language feature (like numeric separators), though they still can't be merged until the new espree is released, because of the github espree reference.

On the other hand, the downside of separate PRs is that we should take care to merge all of them for the same eslint version, even if they have conflicts.

In this particular case, we have three new features - numeric separator, logical assignment, and two new globals which require es2021 environment. At the first glance, it doesn't look there will be any conflicts between the three PRs.

@eslint/eslint-team thoughts about making three separate PRs?

@anikethsaha
Copy link
Member

Thanks, @mdjermanovic for explaining, I missed to notice that even previous PRs were single PRs. My bad!

I would prefer the approach that would help the reviewers and fewer merge conflicts yeah for numeric separators I don't think there would be any conflicting change (at least for the rules ).

Maybe one more way can be to create a branch as numeric separators and separate PRs against numeric separators and then later numeric operators -> master. One drawback of this is more time-consuming. And benefits can be plus points for both separate and single big PR i.e easy to review and it will give a space to fix the merge conflicts (if any) before merging to master !

@kaicataldo
Copy link
Member

I prefer smaller PRs, personally :) As long as folks aren't touching the same code, we should be able to release Espree and then bump the version in individual PRs. When they're all ready and approved, we can merge them in, rebasing the latter ones on the first.

@yeonjuan
Copy link
Member

@mdjermanovic

thoughts about making three separate PRs?

Personally I prefer separate PR, cause it's easy to review and avoid conflicts

@nzakas
Copy link
Member

nzakas commented Aug 17, 2020

I also prefer smaller PRs. Larger ones make conflicts more likely and so can be more difficult to get it merged. I’d actually prefer merging Espree changes first and then having a separate PR for each rule change.

@mdjermanovic
Copy link
Member Author

Seems there's an agreement to split PRs.

I'm working on a PR for the rest of the changes related to numeric separators.

nzakas pushed a commit that referenced this issue Aug 24, 2020
* Upgrade: espree@7.3.0

* update docs

* Update types.js
btmills added a commit that referenced this issue Aug 28, 2020
This adds tests for numeric literal object keys that aren't digit-only
decimal integer literals, including ES6 binary and octal literals,
ES2020 bigint literals, and ES2021 numeric separators. The valid tests
have similar-looking literals that are distinct keys, and the invalid
tests have differently-formatted literals that result in duplicate keys.
kaicataldo pushed a commit that referenced this issue Aug 29, 2020
… (#13574)

* Update: support numeric-separator in no-loss-of-precision (refs #13568)

* Chore: changed the function jsdocs description

* Chore: removed extra getRaw calls

* Chore: update lib/rules/no-loss-of-precision.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
kaicataldo pushed a commit that referenced this issue Aug 29, 2020
* update astUtils.isDecimalInteger and astUtils.isDecimalIntegerNumericToken

* add tests and comment for prefer-numeric-literals

* add tests for quote-props
kaicataldo pushed a commit that referenced this issue Aug 29, 2020
This adds tests for numeric literal object keys that aren't digit-only
decimal integer literals, including ES6 binary and octal literals,
ES2020 bigint literals, and ES2021 numeric separators. The valid tests
have similar-looking literals that are distinct keys, and the invalid
tests have differently-formatted literals that result in duplicate keys.
@mdjermanovic
Copy link
Member Author

This is all merged and released, so closing.

@naoisegolden
Copy link

Anyone looking for the rule enforce or prefer numeric separators: I started a simple version here https://github.com/naoisegolden/eslint-plugin-prefer-numeric-separators, feel free to contribute.

@pustovalov
Copy link

Also you can try this rule: unicorn/numeric-separators-style

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Mar 18, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features new syntax This issue is related to new syntax that has reached stage 4 rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

7 participants