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

Add relative indexing method proposal #889

Merged
merged 8 commits into from Nov 24, 2020
Merged

Add relative indexing method proposal #889

merged 8 commits into from Nov 24, 2020

Conversation

@zloirock
Copy link
Owner

@zloirock zloirock commented Nov 15, 2020

@zloirock zloirock force-pushed the item-method branch from 5f881ea to b431ce0 Nov 21, 2020
@zloirock zloirock changed the title Add `.item` method proposal Add relative indexing method proposal Nov 21, 2020
@zloirock
Copy link
Owner Author

@zloirock zloirock commented Nov 22, 2020

@syg @tabatkins @mathiasbynens hey, how do you think, makes it sense to add this proposal to core-js with the current name / in the current form? The conflict with another (already obsolete) proposal, possible problems and solutions see here.

@zloirock zloirock force-pushed the item-method branch 2 times, most recently from 31400c6 to c9a03d7 Nov 22, 2020
@mathiasbynens
Copy link

@mathiasbynens mathiasbynens commented Nov 23, 2020

Yes, the old proposal should not be considered at all IMHO.

@zloirock
Copy link
Owner Author

@zloirock zloirock commented Nov 23, 2020

@mathiasbynens sure, in this case, core-js will use String#at from the new proposal in the next major version. But the problem here is that for many years was available the proposal with code points semantic, most likely someone uses one of our polyfills in production (billions of core-js copies with this polyfill for 6+ years - it's too long, and not all of the users understand what means stage 0 proposals), and after adding the method with the same name and code units semantic it could break the web. So, I think, here could help or usage code points semantic (I don't think that many users used the old proposal with negative indexes), or changing the name. Or just ignore it, save it as is, and wait for the feedback from the early browser implementations?

@syg
Copy link

@syg syg commented Nov 23, 2020

Hm, this is indeed a risk with shipping early-stage proposals. I also think the old proposal should be removed in favor of the new one. A method named at with code point semantics, however, is explicitly rejected as part of the current relative indexing proposal, to be consistent with other indexables.

To make sure I understand the risk, core-js's old polyfill for the old proposal only adds String#at if one doesn't already exist, so if one suddenly existed (because engines shipped it), then that might break? If the browsers get such bug reports, then the next name to try is itemAt, but I'd like to wait for the bug reports first.

@zloirock
Copy link
Owner Author

@zloirock zloirock commented Nov 23, 2020

To make sure I understand the risk, core-js's old polyfill for the old proposal only adds String#at if one doesn't already exist, so if one suddenly existed (because engines shipped it), then that might break?

Yep, without any additional code units / code points feature detection since I didn't think that in the future the method with this name will be added with code units semantic. I'll add it in the new patch releases, however, there are many projects where the old core-js version will not be updated.

@zloirock zloirock force-pushed the item-method branch from c9a03d7 to 9c07cee Nov 24, 2020
@zloirock zloirock merged commit 5a1efc1 into master Nov 24, 2020
20 checks passed
20 checks passed
eslint (ubuntu-latest, 14.x)
Details
eslint (ubuntu-latest, 14.x)
Details
tests (ubuntu-latest, 10.13)
Details
tests (ubuntu-latest, 10.13)
Details
tests (ubuntu-latest, 12.13)
Details
tests (ubuntu-latest, 12.13)
Details
tests (ubuntu-latest, 14.15)
Details
tests (ubuntu-latest, 14.15)
Details
tests (windows-latest, 10.13)
Details
tests (windows-latest, 10.13)
Details
tests (windows-latest, 12.13)
Details
tests (windows-latest, 12.13)
Details
tests (windows-latest, 14.15)
Details
tests (windows-latest, 14.15)
Details
tests (macos-latest, 10.13)
Details
tests (macos-latest, 10.13)
Details
tests (macos-latest, 12.13)
Details
tests (macos-latest, 12.13)
Details
tests (macos-latest, 14.15)
Details
tests (macos-latest, 14.15)
Details
@zloirock zloirock deleted the item-method branch Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.