Eliminate dependencies on jquery, and underscore.#6
Eliminate dependencies on jquery, and underscore.#6gunn wants to merge 3 commits intometeorhacks:masterfrom
Conversation
|
Also, it would be nice if we could do away with underscore on the server. Could the template code be replaced with something as simple as this? var injectHtml = '<script type="text/inject-data">' + data + '</script>'; |
|
I'm also okay with removing underscore from the server as well. Go for it and add it to here as well. |
|
@arunoda: done. I haven't done any testing however. Can you tell me a good way to run the tests? |
|
We have some tests which covers most of the cases. |
|
@arunoda Haha, previously I'd tried |
| var res = HTTP.get(url); | ||
|
|
||
| var matched = res.content.match(/data">(.*)<\/script/); | ||
| var matched = res.content.match(/data">(.*?)<\/script/); |
There was a problem hiding this comment.
The query makes it non-greedy. Previously it was matching additional tags. See http://regexr.com/3cu6c
|
Hey @gunn great shout on removing jQuery - I've had a local fork without jQuery for a while and kept meaning to do a PR! Just curious - you mentioned about about using And I'm hoping I don't have a bug lurking about that I'm not aware of! Thanks :) |
|
@tomwasd no problems, just being cautious as I don't what browser versions we need to support. See http://caniuse.com/queryselector I was going to point out someone had already had trouble with .trim() in IE at #3 but... 😛 |
|
Thanks @gunn Ha I now use the lovely es5-shim package internally so I can be confident .trim() is present Next step is removing jQuery from Kadira... 😮 |
|
Any news on this PR? |
I don't think we should need all of jquery just for a couple of methods.
We could simplify this with
document.querySelectorif we don't care about IE <= 7.I don't think underscore is used on the client, so I've removed that dependency too.
Does this look good?