Skip to content

Adding promise polyfill#274

Merged
rtibbles merged 1 commit intolearningequality:developfrom
DXCanas:promise-polyfill
May 10, 2017
Merged

Adding promise polyfill#274
rtibbles merged 1 commit intolearningequality:developfrom
DXCanas:promise-polyfill

Conversation

@DXCanas
Copy link
Copy Markdown
Member

@DXCanas DXCanas commented May 9, 2017

Using this package, promise support goes back to IE8.

Was going to try and employ this technique so that we wouldn't even load the code, but it seemed like it might be a little messy to write a script outside of base.js (already dedicated to preparing the environment). @rtibbles thoughts? and if I do employ the implementation he suggests, where should I put that code? inside a <script> tag in base.html?

@DXCanas DXCanas requested review from jayoshih and rtibbles May 9, 2017 01:02
Copy link
Copy Markdown
Member Author

@DXCanas DXCanas left a comment

Choose a reason for hiding this comment

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

My ESLint atom package came in to clean up a little bit when I saved. Figured it wouldn't do any harm.

var get_cookie = require("utils/get_cookie");
var $ = require('jquery');
var get_cookie = require('utils/get_cookie');
global.$ = $;

This comment was marked as spam.

This comment was marked as spam.

Copy link
Copy Markdown
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

This is fine as is - I don't think the performance implications are significantly problematic that we need to async the polyfill.

var get_cookie = require("utils/get_cookie");
var $ = require('jquery');
var get_cookie = require('utils/get_cookie');
global.$ = $;

This comment was marked as spam.

@rtibbles rtibbles merged commit b878ac4 into learningequality:develop May 10, 2017
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