karma-runner / karma-dojo

A Karma plugin. Adapter for Dojo Toolkit.
MIT License
7 stars 10 forks source link

Update sourceURL? #6

Open jtmille3 opened 9 years ago

jtmille3 commented 9 years ago

Chrome updated it's regex around September 2014. It no longer matches the output from dojo.js:319. Trying to debug code is hard without this change.

Please see: https://bugs.dojotoolkit.org/ticket/18248#no1

The fix is simple. Change lib/dojo.js line 319 to.

return eval_(text + "\r\n//# sourceURL=" + hint);

tomwayson commented 9 years ago

probably a better fix would be to pull a more up to date copy of dojo.js from the dojo source. That ticket makes me realize that the copy of dojo.js in this repo is pre-1.7.5 and Dojo is now on 1.10.x

I tried to make it so you could choose the location of your own configuration file in #5, but have had no response and haven't had time to get back to it.

tomwayson commented 9 years ago

btw - another idea I had would be to remove dojo.js from this repo and include the dojo package as a dependency to this repo. That's the way the Intern works. The only downside being that you get a lot more than just dojo.js, but then this repo wouldn't be responsible for maintaining a copy of that file.

Other Karma framework plugins like karma-requirejs use a peer dependency, but I think it would be better in this case to follow the Intern's model and use a regular nested dependency since it is common practice to use a dojo loader from the CDN and the parent project might not need to include Dojo.

I think it would be a good idea to do one or the other (either update dojo.js or include dojo as a dependency). Either should be compatible with what I want to do in #5.

@jtmille3 - do you (or anyone else) have any thoughts on which would be better? I'm leaning towards including as a dependency.

jtmille3 commented 9 years ago

A dependency makes more sense, but then you might have migration issues to maintain between releases. I think it would be worth the risk. Let me know if you want help updating your fork if this repo has gone stagnant.

tomwayson commented 9 years ago

Cool. I was planning to PR here first and see how that goes.

I wasn't going to get to this until next week at the earliest, maybe later, so if you end up jumping in first I'll help out in any way that I can.

jtmille3 commented 9 years ago

I submitted a pull request. Otherwise my fork is at https://github.com/karma-runner/karma-dojo and is submitted to npm as dojo-karma-wrapper. Cheers.

tomwayson commented 9 years ago

@jtmille3 awesome!

I'll pull it down, try it out, and report back.