search

Rosey / markdown-draft-js

A tool to convert content created in DraftJS to markdown and vice versa.
https://rosey.github.io/markdown-draft-js/
MIT License
318 stars 70 forks source link

Can't resolve 'babel-polyfill' #23

Closed juliankrispel closed 7 years ago

juliankrispel commented 7 years ago

babel-polyfill seems to be a required dependency but not defined as a peerdep in the package.json

Do we really need babel-polyfill for this package?

Rosey commented 7 years ago

Shoot :/

At a glance, right now it does use reverse and forEach. Thinking though it would at the very least be better to change this to just require those two specific shims.

Rosey commented 7 years ago

Perhaps even better would be a refactor to not need any shims? I'm wondering if there are best practices for libraries like this, obviously the less crap the better, but does that mean we're never allowed to use any new(ish) JS methods in a library?

I see draft-js-plugins uses es2015 methods but advises requiring babel-polyfill separately.

So maybe that's the way to go.

Sorry, thinking out loud :) Leaning towards copying draft-js-plugins's approach at this point though. Any thoughts?

juliankrispel commented 7 years ago

My recommendation would be - recommend it's usage but don't explicitly require it 👍 - the package user wouldn't have to install it if they don't need/want it...

Again - Thanks for replying so quick - you bear the markings of an excellent open sourcerer 🙌 On Thu, Sep 21, 2017 at 4:33 AM Rose notifications@github.com wrote:

Perhaps even better would be a refactor to not need any shims? I'm wondering if there are best practices for libraries like this, obviously the less crap the better, but does that mean we're never allowed to use any new(ish) JS methods in a library?

I see draft-js-plugins uses es2015 methods but advises requiring babel-polyfill separately https://github.com/draft-js-plugins/draft-js-plugins/blob/master/FAQ.md#the-editor-throws-errors-in-internet-explorer-11 .

So maybe that's the way to go.

Sorry, thinking out loud :) Leaning towards copying draft-js-plugins's approach at this point though. Any thoughts?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Rosey/markdown-draft-js/issues/23#issuecomment-331041467, or mute the thread https://github.com/notifications/unsubscribe-auth/ABIhWiqFmeKLGL38WZed9H8QB9-1H3iaks5skdj7gaJpZM4PelUW .

Rosey commented 7 years ago

Thanks for pointing this out & advising! I'll look into making the change.... soon. ha. Not sure exactly when :) Now you have me wondering about another dependency that I can't remember if it's actually needed - markdown. (eg do we need markdown and remarkable or did I include markdown early on before I discovered remarkable? Can't remember.) So want to check in on that too.

juliankrispel commented 7 years ago

@Rosey I believe it was the latter (you replaced markdown with remarkable and forgot about it) - Everybody does it btw, I add and forget about them all the time - Wouldn't it be nice if there was a tool which automatically does this for us