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

do not modify String.prototype in js package #4200

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

jharris4
Copy link
Contributor

No description provided.

@jharris4
Copy link
Contributor Author

Not sure if THIRD-PARTY-NOTICES.txt should be updated/deleted to reflect the changes in this PR...?

Signed-off-by: Jon Harris <harris.jb@gmail.com>
@@ -4,8 +4,6 @@
*/

import Token from './Token.js';
import './polyfills/codepointat.js';
Copy link
Contributor

@ericvergnaud ericvergnaud Mar 21, 2023

Choose a reason for hiding this comment

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

how does removing this relate to the PR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those polyfills are modifying the String.prototype if you look at the code.

For example here:

https://github.com/antlr/antlr4/blob/master/runtime/JavaScript/src/antlr4/polyfills/codepointat.js#L58

I did check that these polyfills are no longer needed, we've come a long way since they were added around 6 years ago! Now those functions that were polyfilled are available everywhere.

https://caniuse.com/mdn-javascript_builtins_string_codepointat
https://caniuse.com/mdn-javascript_builtins_string_fromcodepoint

@ericvergnaud
Copy link
Contributor

Yes THIRD-PARTY-NOTICES should be removed

Signed-off-by: Jon Harris <harris.jb@gmail.com>
@ericvergnaud
Copy link
Contributor

@parrt blessed

@parrt parrt added this to the 4.12.1 milestone Mar 22, 2023
@parrt parrt merged commit 1bc7a45 into antlr:dev Mar 22, 2023
jimidle pushed a commit to jimidle/antlr4 that referenced this pull request Mar 28, 2023
* do not modify String.prototype in js package

Signed-off-by: Jon Harris <harris.jb@gmail.com>

* remove notice file that is no longer relevant

Signed-off-by: Jon Harris <harris.jb@gmail.com>

---------

Signed-off-by: Jon Harris <harris.jb@gmail.com>
Signed-off-by: Jim.Idle <jimi@idle.ws>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants