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.

Sign up for GitHub

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

Jump to bottom

fix: remove string.repeat for ie11 #1772

Merged
merged 1 commit into from Oct 21, 2020
Merged

fix: remove string.repeat for ie11 #1772

merged 1 commit into from Oct 21, 2020

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Oct 6, 2020

Marked version: 1.2.0

Description

string.repeat is not a function in IE11. If we are still supporting IE11 then we need to create our own repeat helper function. I searched for the fastest function and found https://stackoverflow.com/a/5450113/806777.

I'm not exactly sure how we can write a test to make sure string.repeat isn't used in the future since even node v4 supports string.repeat.

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
  • no tests required for this PR.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

@vercel
Copy link

vercel bot commented Oct 6, 2020 •
edited

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/markedjs/markedjs/kouc9ky5a
✅ Preview: https://markedjs-git-b71729108320cd7725faf9b52f414e24d3857f19.markedjs.vercel.app

styfle
styfle approved these changes Oct 6, 2020
View reviewed changes
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Seems fine.

When do we want to drop support for IE 11?

@UziTech
Copy link
Member Author

UziTech commented Oct 6, 2020

When do we want to drop support for IE 11?

Not sure, but I don't think we need to yet.

@styfle
Copy link
Member

styfle commented Oct 6, 2020

We should settle on a date to drop support for IE since we are not testing it and even Microsoft products are dropping support.

timeline

View Source

How about November 30?

@UziTech
Copy link
Member Author

UziTech commented Oct 6, 2020

Sure. I'm good with it whenever but it will be a breaking change.

Should we be removing the es5 version as well since we will only be supporting es6+ browsers?

@styfle
Copy link
Member

styfle commented Oct 6, 2020

Sounds good to me. Grouping a few breaking changes is preferable to me 👍

@UziTech
Copy link
Member Author

UziTech commented Oct 21, 2020

I'm going to merge this to see if semantic release will pick up the correct version

@UziTech UziTech changed the title fix: fix string.repeat for ie11 fix: remove string.repeat for ie11 Oct 21, 2020
@UziTech UziTech merged commit 2707070 into markedjs:master Oct 21, 2020
@UziTech UziTech deleted the repeat-string branch October 21, 2020 14:56
github-actions bot pushed a commit that referenced this pull request Oct 21, 2020
@semantic-release-bot
chore(release): 1.2.2 [skip ci]
2f45eec 
## [1.2.2](v1.2.1...v1.2.2) (2020-10-21)

### Bug Fixes

* remove string.repeat for ie11 ([#1772](#1772)) ([2707070](2707070))
@github-actions
Copy link

🎉 This PR is included in version 1.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@styfle styfle styfle approved these changes

@davisjam davisjam Awaiting requested review from davisjam

@joshbruce joshbruce Awaiting requested review from joshbruce

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Object doesn't support property or method 'repeat'
2 participants
@UziTech @styfle