search

BorisMoore / jquery-tmpl

The original official jQuery Templates plugin. This project was maintained by the jQuery team as an official jQuery plugin. It is no longer in active development, and has been superseded by JsRender.
3.23k stars 1.01k forks source link

Add AMD definitions #182

Closed djessup closed 5 years ago

djessup commented 9 years ago

I realise this plugin is no longer under active development, however it is still useful today.

I've created a patch to include AMD module definitions to make it more compatible with modern deployments using RequireJS or similar without needing ugly shim code.

The functionality is completely unchanged, and should be 100% backwards compatible. The only difference is the plug defines itself as an AMD module in supported environments.

BorisMoore commented 9 years ago

Thanks - yes, makes sense to do this. I will try to verify and pull within a few days...

djessup commented 9 years ago

Thanks Boris, let me know if there is anything else I can do to help.

BorisMoore commented 9 years ago

Yes, sure I'll let you know. I didn't manage to get to this yet, but not forgotten. Hopefully soon. (If it is more than a week more, feel free to ping me :)

BorisMoore commented 9 years ago

I'm looking at AMD and Node.js support, as appropriate, in JsRender and JsViews, along with the jquery-tmpl case, so I'm working on them all in parallel, for a consistent approach. For jquery-tmpl and JsViews we need jQuery and the DOM - so node.js on the server will not be a scenario. So I think we can remove CommonJS support other than for JsRender. Does that make sense?

djessup commented 9 years ago

Sorry! I completely missed these messages.

The code in the PR is just standard boilerplate loader code and I can't see any reason why your modified version wouldn't work just fine. Would you like me to update the PR?

BorisMoore commented 9 years ago

Yes, you could do that. Thanks...

djessup commented 9 years ago

Ok, CommonJS has been removed from the factory method - sorry I've been so slow with updates! Let me know if there is anything else you think needs attention to merge.

BorisMoore commented 9 years ago

Thanks. I have merged the changes locally, but need to do a sanity check / run tests, before pushing. Unfortunately I am heading out tomorrow for a trip abroad, and may not be able to complete the pull for the moment - until returning... If I can get to it sooner, I will...

djessup commented 9 years ago

No problem, I'm sure it can wait a little bit longer :-)

I did run the existing unit tests on both the raw and minified code, there are a few that it fails, but they also fail it the current non-AMD version so I figured they were just a little out of date.