Skip to content
This repository has been archived by the owner on May 14, 2020. It is now read-only.

Replaced ES6 instance methods with static one #76

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

soulman-is-good
Copy link

@soulman-is-good soulman-is-good commented Apr 9, 2017

What is done in this PR

  • Replaced ES6 insance methods Array.prototype.find with Array.find as babel-plugin-transform-runtime does not polyfill them in such a way (see why and also see readme from core-js, without global scope pollution).
  • Updated sortObjectKeys handling according to MDN

Currently react-json-tree doesn't work on IE and Edge 11, see MDN

@alexkuz please consider this changes.

@alexkuz
Copy link
Owner

alexkuz commented Apr 9, 2017

Thank you! But I'd just use a polyfill actually

import 'core-js/fn/array/find';

@soulman-is-good
Copy link
Author

importing this will pollute global scope by modifying Array.prototype directly. But since you are using transform-runtime (and I think this is the right way) the es6 instance method should be written in static way.
Please @alexkuz consider my answer, I want to use your lib badly, but I have to support IE9+ )

@soulman-is-good
Copy link
Author

soulman-is-good commented Apr 9, 2017

And I would also include fix for this issue in this PR

image

Can you give a hint, what you are waiting sortObjectKeys should be?

Bad question. Description is on the README. So I'll just update this file too

@alexkuz
Copy link
Owner

alexkuz commented Apr 9, 2017

I think the best solution here would be not using find at all, I'll get rid of it in the next version. For now, if you're using it in an app, just add a polyfill.

@alexkuz
Copy link
Owner

alexkuz commented Apr 9, 2017

(Array.find doesn't seem to me like an appropriate solution, it's not in specs so technically this code is not valid)

@wdcryer
Copy link

wdcryer commented Dec 19, 2017

It would be great to get the sortObjectKeys change merged in, at least. The component breaks when sortObjectTrees is simply set to true.

@gitgrimbo
Copy link

Hi, can this PR be merged/released?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants