Skip to content

Conversation

john-ko
Copy link
Owner

@john-ko john-ko commented Feb 29, 2020

  • Updated README.md
  • Changed unit tests from mocha to jest
  • Updated test to get 100% coverage
  • Updated syntax and make more readable
  • Cleaned up code
  • Removed unused npm scripts and dev deps.

@coveralls
Copy link

coveralls commented Feb 29, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 5b08ae7 on dev into a4bb7d3 on master.

@@ -1,87 +0,0 @@
import loadScript from '../../src/index'
Copy link
Owner Author

Choose a reason for hiding this comment

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

dangit i forgot the main script

README.md Outdated
- https://bitsofco.de/async-vs-defer/
- https://flaviocopes.com/javascript-async-defer/
- will fallback to whichever is Best practice
- also since this script is most likely loaded on content load and scriptloading is done after, might not matter? should test.
Copy link

@nicohsieh nicohsieh Feb 29, 2020

Choose a reason for hiding this comment

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

Did some research and realized that dynamic script are async by default! https://javascript.info/script-async-defer#dynamic-scripts Looks like we don't even have to add it~! Maybe it can be a patch later, if so I can create an issue for it 😁

@john-ko john-ko linked an issue Mar 1, 2020 that may be closed by this pull request
@john-ko john-ko merged commit 0d8bb6f into master Mar 1, 2020
@john-ko john-ko deleted the dev branch March 1, 2020 19:14
@nicohsieh
Copy link

LGTM!

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.

async vs defer

3 participants