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

remove ES6 syntax from getElementsByClassName #91

Merged
merged 1 commit into from Aug 12, 2020

Conversation

rg1
Copy link
Contributor

@rg1 rg1 commented Jul 15, 2020

The new method getElementsByClassName uses ES6 syntax. This is a problem when trying to polyfill an older engine.

the new method getElementsByClassName uses ES6 syntax. This is a problem when trying to polyfill an older engine.
@brodybits
Copy link
Member

Thanks. Can you give us an idea what kind of engine or browser you need this for?

In my own experience, these changes could help on Android 4.4, which is extremely old and losing support from toolkits such as Apache Cordova.

If we do accept this proposal, it would be nice to configure eslint to reject the ES6 syntax (maybe part of PR #90).

/cc @karfau @kethinov @awwright

@awwright
Copy link
Contributor

I was just going to chime in: if we standardize on supported platforms, this should include an eslint rule for it.

If this is the only thing stopping use in a handful of platforms, +1. I'm not worried about syntax sugar, I'm more worried about getting to use optimizations like Set and Map.

@karfau
Copy link
Member

karfau commented Jul 17, 2020

With the massive amount of usage out there, introducing a syntax that is not supported is a breaking change. So we should really define what platforms to support (and for how long).

I think this PR is good as a first step (didn't check the code details, just talking about the intention here). Next one should be to

  1. define the supported platforms

  2. take measures to prevent future breaking changes by one or multiple of

    • run tests on them if possible
    • eslint rules to prevent unsupported syntax (not sure how much it can cover)
    • offer a polyfilled version
  3. When introducing things that are breaking but can be polyfilled, document what features need to be polyfilled (but it would still be a breaking change)

@rg1
Copy link
Contributor Author

rg1 commented Jul 17, 2020

The platform in question is Jint.

The changed function is the only place that is currently using modern syntax and with my change, I can use xmldom even in the old/stable branch.

For the record, the other platform I know of that won't support this syntax is IE11.

@xqdoo00o
Copy link

I have the IE11 error too, and fixed as this commit said. It's a little weird only this function but no other funcs use es6 syntax.

Copy link
Member

@karfau karfau left a comment

Choose a reason for hiding this comment

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

I agree we should have some linting for this, but I think we should land this fix/PR to unblock the people affected.

@brodybits brodybits merged commit 66db3e4 into xmldom:master Aug 12, 2020
brodybits added a commit to brodybits/xmldom that referenced this pull request Aug 17, 2020
should guard against newer ES6 syntax that could lead to issues with
some ES5 environments such as:

- Jint - https://github.com/sebastienros/jint
- Duktape
- IE11
- Android pre-5.0 WebView (already past EOL)

as discussed in PR xmldom#91 - xmldom#91
brodybits added a commit to brodybits/xmldom that referenced this pull request Aug 17, 2020
should guard against newer ES6 syntax that could lead to issues with
some ES5 environments such as:

- Jint - https://github.com/sebastienros/jint
- Duktape
- IE11
- Android pre-5.0 WebView (already past EOL)

as discussed in PR xmldom#91 - xmldom#91
brodybits added a commit to brodybits/xmldom that referenced this pull request Aug 18, 2020
should guard against newer ES6 syntax that could lead to issues with
some ES5 environments such as:

- Jint - https://github.com/sebastienros/jint
- Duktape
- IE11
- Android pre-5.0 WebView (already past EOL)

as discussed in PR xmldom#91 - xmldom#91
brodybits added a commit that referenced this pull request Aug 18, 2020
should guard against newer ES6 syntax that could lead to issues with
some ES5 environments such as:

- Jint - https://github.com/sebastienros/jint
- Duktape
- IE11
- Android pre-5.0 WebView (already past EOL)

as discussed in PR #91 - #91
This was referenced Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants