-
Notifications
You must be signed in to change notification settings - Fork 1
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
[RELEASE] v0.0.5 #11
Merged
Merged
[RELEASE] v0.0.5 #11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes part of #8 Allow the passing of array of values to run once polyfills have been loaded.
Allow array to be passed for: * `features` (parameter 1) * `scriptURL` (parameter 2) * `initFunction` (parameter 3) You can still pass strings if you want though
It thought it could stay here... it couldn't!
Fix try/catch for init functions
Debugging/troubleshooting/fun messages added to each step, so you know where things go wrong - based on the new version of dynamicPolyfill.js
Allow arrays for all parameters Try catch Minification and uglification
willstocks
added
bug
Something isn't working
enhancement
Not a bug, but not a new feature - an improvement to what exists
feature request
Functionality that doesn't currently exist, but could/should!
labels
Feb 5, 2019
Updated parameter details Updated example scripts Updated stats
If the function wasn't being passed as an array, it wouldn't execute. Fixed now!
`fns` in `loadPolyfill` was unused... not sure how it even got there!
`initialiseMyScript` had an unnecessary `var` (`var fn = new Function`) which has been instead moved inline into the `try{}` Also reordered functions so they are in a more "vertically readable" format (they now are ordered logically!)
1) Iterate through array values using `Array.forEach()` 2) Split out `loadMyScript` repeated code into a helper function - minimising duplication! 3) `initialiseMyScript` is no longer called from the "core" `pageLoaded` function - instead it is called depending on whether the scripts that are to be used have loaded or not. 4) Inline comments to explain what each step is doing 5) Retain backwards compatibility!
Pain to maintain and generally unuseful unless doing in-depth troubleshooting (at which point, you'd add your own `console.log`s).
Refactored init function to include a micro-helper function to avoid duplication!
13 tasks
Not only did I forget a commit message/details on the last commit (sorry - this is simply making up for that - no code changes!), but also... I realised I had two functions that were simply there to call other functions. They weren't doing anything for themselves! Therefore, `function dynamicPolyfill`, `function pageLoaded` and `function checkNativeSupport` have now all been combined into a single, function: `function dynamicPolyfill` Also as part of this commit - support for Safari and it's weird behaviours. Finally, some general tidying up of code Now all I need to do is re-add the inline comments?
"Local" and CDN deployment now included!
Updated PR message with recent changes. Ready to merge 🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
Something isn't working
enhancement
Not a bug, but not a new feature - an improvement to what exists
feature request
Functionality that doesn't currently exist, but could/should!
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
README
Updated parameter details
Updated example scripts
Updated deployment instructions (include "local")
Updated stats
The actual scripts
Major overhaul:
function dynamicPolyfill
,function pageLoaded
andfunction checkNativeSupport
have now all been combined into a single, function:function dynamicPolyfill
- closes [Enhancement] checkNative if statement could be better #14Array.forEach()
loadMyScript
repeated code into a helper function - minimising duplication! See [Review] Optimal performance & supportability #12.initialiseMyScript
is no longer called from the "core"pageLoaded
function - instead it is called depending on whether the scripts that are to be used have loaded or not. See [Review] Optimal performance & supportability #12.initialiseMyScript
had an unnecessaryvar
(var fn = new Function
) which has been instead moved inline into thetry{}
. See [Review] Optimal performance & supportability #12.fns
inloadPolyfill
was unused... not sure how it even got there! See [Review] Optimal performance & supportability #12.Previous beta's:
Fix erroneous
try{}catch{}
Remove
console.log
from dynamicPolyfill.js that shouldn't have remainedAdd various
console
messages to dynamicpolyfill-consolemessages.js for troubleshooting/debugging/fun purposes [NOW DEPRECATED]Allow array to be passed for:
features
(parameter 1)scriptURL
(parameter 2)initFunction
(parameter 3)You can still pass simple strings to each if you want though
Closes [Feature Request] Also allow array of "initialisation" functions! #10 and closes [Feature Request] Allow array of scripts to load #8
Fix issue where if
initFunction
wasn't an array, it wouldn't executeDeprecated
dynamicpolyfill-consolemessages.js
due to complexity to maintain (if you really want it - log an issue and I'll re-add it!)