add polyfill for ChildNode remove #21797
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💚 Build Succeeded |
💔 Build Failed |
|
x-pack flaky reporting test jenkins, test this |
💔 Build Failed |
💔 Build Failed |
jbudz
reviewed
Aug 8, 2018
webpackShims/childnode-remove.js
Outdated
| * under the License. | ||
| */ | ||
|
|
||
| const childNodeRemove = require('childnode-remove'); |
There was a problem hiding this comment.
my bad, const childNodeRemove = require('../node_modules/childnode-remove');
jbudz
reviewed
Aug 8, 2018
webpackShims/childnode-remove.js
Outdated
|
|
||
| const childNodeRemove = require('childnode-remove'); | ||
|
|
||
| childNodeRemove.polyfill(); |
There was a problem hiding this comment.
I took a quick look at the lib, it doesn't check if remove is already defined. I'd do some light feature detection if possible, native code will most likely be quicker.
💔 Build Failed |
nreese
added
blocker
Team:Operations
|
jenkins, test this |
💚 Build Succeeded |
jbudz
reviewed
Aug 9, 2018
webpackShims/childnode-remove.js
Outdated
|
|
||
| /* eslint-disable */ | ||
|
|
||
| // https://opensource.org/licenses/MIT |
There was a problem hiding this comment.
you mean put the MIT license inline?
💔 Build Failed |
|
jenkins, test this |
💔 Build Failed |
💔 Build Failed |
|
jenkins, test this |
💔 Build Failed |
LeeDr
approved these changes
Aug 10, 2018
There was a problem hiding this comment.
I tested these changes in IE 11 on Windows and it resolved the Dashboard problems. I could Add NEw Visualization and I could Remove a visualization from a dashboard without errors. There are some warnings in the console but they didn't impact functionality.
I did not review the code changes.
jbudz
reviewed
Aug 10, 2018
webpackShims/childnode-remove.js
Outdated
| * | ||
| * The MIT License (MIT) | ||
| * | ||
| * Copyright (c) 2011-2015 Twitter, Inc |
There was a problem hiding this comment.
can we update this to Copyright (c) 2016-present, jszhou with a link to the github repo?
|
Can we suffix the file with -polyfill to mostly follow the others? |
|
Can we add this to tests_entry_template.js too? LGTM after those 3, feel free to merge after green |
💔 Build Failed |
💔 Build Failed |
|
@jbudz CI keeps failing with Do you know where this is coming from? |
|
ran |
💚 Build Succeeded |
nreese
added a commit
to nreese/kibana
that referenced
this pull request
Aug 10, 2018
* add polyfill for ChildNode remove * call polyfill function * add files as suggested by jbudz * remove checks around polyfill * just use polyfill that mozilla recommends * add MIT license to file * suffix file with polyfill, update copyright and link to repo * update notice.txt
nreese
added a commit
to nreese/kibana
that referenced
this pull request
Aug 10, 2018
* add polyfill for ChildNode remove * call polyfill function * add files as suggested by jbudz * remove checks around polyfill * just use polyfill that mozilla recommends * add MIT license to file * suffix file with polyfill, update copyright and link to repo * update notice.txt
nreese
added a commit
that referenced
this pull request
Aug 10, 2018
* add polyfill for ChildNode remove * call polyfill function * add files as suggested by jbudz * remove checks around polyfill * just use polyfill that mozilla recommends * add MIT license to file * suffix file with polyfill, update copyright and link to repo * update notice.txt
nreese
added a commit
that referenced
this pull request
Aug 10, 2018
* add polyfill for ChildNode remove * call polyfill function * add files as suggested by jbudz * remove checks around polyfill * just use polyfill that mozilla recommends * add MIT license to file * suffix file with polyfill, update copyright and link to repo * update notice.txt
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #21762
Adds polyfill for ChildNode.remove to support IE11