Added server-side node compatibility (take 2)

[oliverfoster]

67

Works undertaken

• Upgraded grunt
• Added umd wrapper and grunt code to produce a umd module
• Shimmed window.location in node
• Fixed a few scope issues, missing variable declarations and global scope creep
• Remove text-encoding and replaced with a simpler polyfil
• Added a url parser to distinguish between node and browser apis
• Added a node shim forXMLHttpRequest
• Changed test readme
• Fixed old test for ADL.xapiutil
• Added 1 new test for the minified file
• Added intellisense support
• Added source map support

Notes

• This is my second attempt at adding node support apologies for the repeated prs.
• I've tried to add node support with minimal disruption to existing file structures and the inferred utilisation patterns.
• All files should be able to be used individually in the browser via script tags as before.
• I've moved a large set of functions from the top of the xapiwrapper out of the global scope as they shouldn't be declared there.
• I recommend not extending native Date object with Date.prototype.toISOString but instead defining a DateToISOString function.
• I've removed the text-encoding module. This is as it adds support for multiple encoding types when I assume we'll realistically ever need utf8 encoding. For that extra support in internet explorer the module brings a lot of complexity across the node<->browser boundary. I've replaced it with a much simpler utf8 only polyfil from https://gist.github.com/Yaffle/5458286.

  xapiwrapper.min.js

• No longer bundles text-encoding.
• Should now work in requirejs, commonjs and window.ADL global environments.
• Doesn't include a working verson of ADL.launch in node
• Has a sourcemap included

  xapiwrapper.js

• Is the main file for the node package and can be included via const ADL = require('xapiwrapper'); if published on npm.
• Is not minified, contains all of the comments.

  Intellisense

  The functions which wrap each file stop intellisense from working. The wrapping functions are required if a developer is using the files individually in a browser such that the code in each file does not bleed into the global scope. The wrapping functions stop intellisense as they need to be executed by the intellisense interpreter for their modifications to the global ADL namespace to be produced (as far as I can tell). The grunt file now removes these wrapping functions when producing xapiwrapper.min.js and xapiwrapper.js. The grunt file removes these function wrappers as they are supplanted by the umd wrapper function. By removing these functions and producing a separate, unminified file containing all of the comments, intellisense works as it should.

[vbhayden]

We discussed this internally today and are floating the idea of a separate repo for a NodeJS-specific xAPI wrapper. There's a limited functionality one that we use internally, but will let you know after we talk about it more tomorrow morning.

Also, I used your branch this morning and some of the response callbacks were getting dropped when stress testing. Switching to an axios-driven internal wrapper resolved the issue, how are requests being handled for the Node version?

[oliverfoster]

Superb.

how are requests being handled for the Node version?

There is a partial shim for the xmlhttprequest api https://github.com/adlnet/xAPIWrapper/pull/126/files#diff-1b2039be21a4454c52d6b497832996ebR1397 I'm not surprised it drops callbacks, but i'm sure it'd be quite easy to fix with a replacement as you said.

I think the biggest barriers to using this wrapper in node are:

• the separation of the files with the intention of them being used individually, necessitating the wrapping functions #131 #137
• the text-encoding library has node-side behaviour which causes issues when bundling it for the browser then using it in node #136
• specifying browser support and dropping broken browser support (<IE10 doesn't work at the moment) #135
• xapi-launch isn't a server side idea
• http requests (we can sort out a minimum shim for this immediately from the results of your stress test, remove all the <IE10 XDomainRequest stuff and slowly work it into a more cross platform compatible implementation)
• use of window global throughout
• window.location needs to not be used in node
• url parsing needs to switch between environments

Other than that it's pretty nice. It would work in all environments just by changing the scope slightly by always distributing it as a bundled library rather than separate but dependent files.

[oliverfoster]

I really don't mind doing the work, I just need someone to help me figure out an order to submit the prs, a review process and what's possible with the current user requirements and the current state of the code.

Considering that the work required here is pretty straight forward, would it not make sense to use the same well tested wrapper for both environments rather than releasing an entirely new code base?

[FlorianTolk]

A new branch was created for this project.

[oliverfoster]

@FlorianTolk what did you envisage for the NodeCompatibleWrapper branch?

I have series of PR's going into master to move this idea forward. Did you want me to re-base them to the other branch?

I could do with an answer to the question of browser support (#135) and wrapping functions (#137).

[FlorianTolk]

This new branch is meant as a playground where most these new PRs can be directed at, as we do not want to make any major functionality changes to the main. To quote @vbhayden 's response to another PR, the only major changes we want to push to the main branch are:

• directly improve usability at no cost to performance / syntax,
• simply correct typos / bugs,
• preempt future browser deprecation issues with the popular browsers, or
• improve the automated testing

The new branch was created to ingest any other changes members of the community would like to make to the xAPI Wrapper. Thanks!

[oliverfoster]

Thanks for getting back to me. This branch would need to be published to npm to provide standard node support. It'd either need to replace the master publication or be a separate version lineage.

[ptc-alhill]

Can someone comment on the status of this thread? There does not appear to be another branch called NodeCompatibleWrapper and the package (xapiwrapper-node) published by @oliverfoster does not appear to be up to date with his repo (the github page for it has disappeared too)

[oliverfoster]

Well. My attempt failed. There was too much fear that dropping support for older browsers would somehow break the library. Then there was mention of a node version of the wrapper used internally and that it would be released instead of making this one compatible.

Nothing much happened after that and I gave up.

[ptc-alhill]

Thanks for the update. I'll assume that npm package is functionally the same but that your later commits were compatibility oriented