RaveJS / rave

Zero-configuration application bootstrap and development
278 stars 8 forks source link

Auto-sniff for AMD when moduleType is unknown. #46

Closed unscriptable closed 10 years ago

unscriptable commented 10 years ago

In-progress fix for #45.

Initial smoke tests look good. Still needs more unit tests.

unscriptable commented 10 years ago

This code is working pretty well in my tests, atm. However, the file scanning procedure is extremely inefficient since it matches on every line feed. (doh!)

Perhaps I should merge this and then follow-up with a better algorithm (and also add code to avoid regular expressions)?

KidkArolis commented 10 years ago

sounds good to me

KidkArolis commented 10 years ago

I saw your commented out test should ignore text inside RegExps so you probably already know, but if there are quotes in a regex (or I suppose other codeTransitionRxs) the matching stops working, e.g. underscore from bower is affected by this.

But also - since this is now used in findRequires as well - regexes in source might break detection of requires? Or was that always the case?

unscriptable commented 10 years ago

if there are quotes in a regex (or I suppose other codeTransitionRxs) the matching stops working, e.g. underscore from bower is affected by this.

Yep. Lodash is way worse than underscore when it comes to regexps and such. :) I use it for a functional test in cram.js.

I've been meaning to fix RegExps for a while now. Finally getting around to it. :)

unscriptable commented 10 years ago

Ok, I added code to avoid false-positives inside RegExps. It's hard to test every variation of source code that could be thrown at this, but the code could certainly use more tests than I've written. Seems good for now?

unscriptable commented 10 years ago

No longer fails to find the end of a blank string.

LOL. Love that commit message.

Ok, I now have this correctly finding the module type when loading the first module of bootstrap, aerogear-pipeline, jQuery, fastclick, and lodash. Yes, even lodash. :)

unscriptable commented 10 years ago

The RegExp to find AMD defines is the 7th most expensive operation at about 8ms on my tiny project according to Chrome's profiler. That's clearly not a scalable solution on a larger project, but it should allow a much better experience during the bootstrapping phase of app dev.