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

added underscore.string to use startsWith #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jaeh
Copy link
Member

@jaeh jaeh commented Apr 8, 2015

No description provided.

@phaer
Copy link
Contributor

phaer commented Apr 8, 2015

I really thought underscore would include .startsWith(), but no. By design, i think babel should add it to the prototype, because it's in es6 according to MDN.

I am not sure if should add an extra dependency just for one single function which is already included in the language we are targeting. Should we add a babel.transform? Or patch it in ourselves? If we ignore the position argument for now it'd just be

String.prototype.startsWith = String.prototype.startsWith || function startsWith(search) {
  return this.indexOf(search) === 0;
}

right?

@jaeh
Copy link
Member Author

jaeh commented Apr 15, 2015

i looked into this,
babel is not including the ecmascript 5 to 3 polyfills by default.

all in all this bloats a prebuilt inlined html file to around 700k,
compared to 371k without polyfill.

since we are running locally i would not worry about that, we might want to cherrypick those we need later though

file inclusion is in the first line of src/main.js, the path is terrible, but it keeps us from having babel as additional dependency.

@phaer
Copy link
Contributor

phaer commented Apr 16, 2015

Which ECMAScript 5 to 3 polyfills? According to the MDN link posted above, String.prototype.startsWith is only introduced with ES 6.

Anyway, I think the easiest solution for now would be to add our own 1-3 line polyfill.

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.

2 participants