Improving checks for module.exports support

[timeiscoffee]

Current check for module.exports isn't sufficient IMO. Updating to align with checks in other libraries.

[bebraw]

Is this the current spec? If so, I have no problems merging. 👍

Given I don't use this library these days, I wouldn't mind giving contributor rights to it to you. I can still review bits etc.

[timeiscoffee]

@bebraw not sure if there are specs for it, but its what https://github.com/umdjs/umd does. Contrib rights would be awesome and I would really appreciate continued reviews :)

[bebraw]

Yeah, let's go with this then. Do you have any other changes in mind or should I cut a release?

[timeiscoffee]

Tests seems to be broken for me, possibly due to CORS issue, but simple { 'web-security': false } doesn't seem to work.

phantom stdout: NETWORK_ERR: XMLHttpRequest Exception 101: A network error occurred in synchronous requests.

I'm ok with cutting a release now and I can try to fix it later (could also be just my environment).

[bebraw]

Ah, ok. If I remember right, it might need some special env variable. But yeah, I'll cut a release in a bit (within an hour) and get those rights sorted out.

[timeiscoffee]

Awesome, thanks for the prompt replies!

[bebraw]

Included in 0.7.0. I also updated the test infrastructure so it hopefully runs for you now.

[timeiscoffee]

Thanks! I had a very similar changesets on my branch 👍

I noticed that test in amd-with-deps was disabled (line 28 commented out) and `amd-with-deps-and-alias' isn't actually testing for dep & alias. Are these intentional?

[bebraw]

I noticed that test in amd-with-deps was disabled (line 28 commented out) and `amd-with-deps-and-alias' isn't actually testing for dep & alias. Are these intentional?

There might have been some glitches related to those. It has been too long to remember what but the Git log (or blame) might give some insight.

Re-enabling that particular line of code yields

cjs ok
info: browser-with-deps-and-alias ok
info: TypeError: undefined is not a constructor (evaluating 'test()')
info:
info:   phantomjs://code/tmp-48009NyqWvc1PCahn.tmp:58 in testDeps
info:   phantomjs://code/tmp-48009NyqWvc1PCahn.tmp:66
info:   phantomjs://code/tmp-48009NyqWvc1PCahn.tmp:9 in require
info: amd ok
info: browser ok
info: browser-with-deps ok

Most likely there's Require.js missing from the test or so.

Alias would need a better test too most likely.

I wrote the tests just to have a way to run the generated code against different environments. I actually tried pushing the project below umd organization at one point (umdjs/umd#42) but there wasn't interest. Ideally there would be an official test suite and templating implementation.

[timeiscoffee]

Interesting, thanks for the insight!

Shame that integration with umdjs fell apart there. I'll poke around to see if there's anything I can do!