Skip to content
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

Implement as a RequireJS module if available #60

Closed
Rycochet opened this issue Jan 13, 2016 · 6 comments
Closed

Implement as a RequireJS module if available #60

Rycochet opened this issue Jan 13, 2016 · 6 comments

Comments

@Rycochet
Copy link

Rather than forcing the namespace this would let it work properly with modern good practices - there's various patterns, but basically if there's no define function defined then nothing changes, otherwise it should effectively have define(function(){return ADL;}) - but possibly without using the global namespace at all.

@creighton
Copy link
Contributor

Thanks for bringing this up. We'll add it to our todos

@srad
Copy link

srad commented Jan 13, 2016

A colleague just brought this ticket to my attention.

I recently forked your code and created an "AMD" branch and refactored the whole code into requirejs modules to use it in the future in our code.

It also unveiled some design issues and globally leaking variables which I largely fixed.
This could certainly also be made compatible with non-requirejs environtments, since the modules can (and are) compiled into a single minified file: https://github.com/srad/xAPIWrapper/tree/amd

@Rycochet
Copy link
Author

@srad Brilliant timing lol - can't be a straight merge though since you've taken out various required bit of the Gruntfile/package.json (even though the doxstrap usage is broken on Windows) - hopefully most can be incorporated :-)

@zpetterd
Copy link

I've converted it over to a Node module, I still need to develop so comprehensive tests before merging it back into this repo.

You can install it using npm install https://github.com/zapur1/xAPIWrapper.git

It will also attach to the window if not imported using Node JS, instead including as <script> directly from source

@ljwolford
Copy link
Contributor

@zapur1 :
Have there been any updates to a completed node module with tests?

@zpetterd
Copy link

zpetterd commented Mar 8, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants