loadjs add append hook

see my PR for details #40

Mar 18 '17 09:03 sourcec0de

The append hook looks useful but the name feels ambiguous. Did you consider any other names? beforeAppend? insert?

Mar 18 '17 14:03 amorey

@sourcec0de - do you have any thoughts on the naming convention? I'm leaning towards calling the attribute insert instead of append.

Mar 21 '17 15:03 amorey

@amorey I think that makes sense. I can't think of a way to easily indicate that it's overriding the insertion. I kind of like beforeInsert

Mar 21 '17 15:03 sourcec0de

Ok, thanks for the feedback. One problem with beforeInsert is that it isn't clear that the method overrides the DOM insertion method.

Mar 21 '17 15:03 amorey

Sure, in that case i think append makes sense.

Mar 21 '17 15:03 sourcec0de

Once it's decided would you like me to update my PR?

Mar 21 '17 15:03 sourcec0de

Sure, if you can update the PR that would be great. I thought you preferred insert to append...

Mar 21 '17 15:03 amorey

Here's another idea - how about combining the before and append callbacks into one method? If the before callback returns false then we can bypass the default insertion method at L143: https://github.com/muicss/loadjs/blob/master/src/loadjs.js#L143

If we decide to do it this way then I think beforeInsert would be an appropriate name.

Mar 21 '17 15:03 amorey

That's clean!!!!!

On Tue, Mar 21, 2017, 8:56 AM Andres Morey [email protected] wrote:

Here's another idea - how about combining the before and append callbacks into one method? If the before callback returns false then we can bypass the default insertion method at L143: https://github.com/muicss/loadjs/blob/master/src/loadjs.js#L143

If we decide to do it this way then I think beforeInsert would be an appropriate name.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/muicss/loadjs/issues/41#issuecomment-288125070, or mute the thread https://github.com/notifications/unsubscribe-auth/ABgea2dNBOegA7oiiv1_Rh_7McjIQhTZks5rn_MbgaJpZM4MhWVE .

Mar 21 '17 16:03 sourcec0de

Glad you like it! Do you want to modify the PR?

In terms of the name, maybe we should keep before for backwards compatibility.

Mar 21 '17 17:03 amorey

@sourcec0de The latest version of LoadJS (v3.5.0) has support for this functionality. Just return false in the before callback to bypass the default DOM insertion mechanism: https://github.com/muicss/loadjs https://www.npmjs.com/package/loadjs

Let me know if you run into any issues using the new feature.

Mar 28 '17 11:03 amorey