Support for AMD detection

[guybedford]

Currently, Bonzo detects the amd environment using context['amd']. When using systems that need to detect if a module is amd or not, this stops a simple regex (checking for the existence of define.amd in the text) from being able to detect amd properly.

For example, this is used by the Volo package manager to detect if a module needs to have its globals wrapped, which was being triggered by bonzo.

This small change adjusts the define form to support this.

[ded]

they should fix the regex

[ded]

you should send a pr to volo

[guybedford]

Detecting context['define'] == 'function' && context['define']['amd'] with a regex is tricky as context could be anything without digging into the function call. Also if this approach is taken, the regex would have to deal with a lot of different variations. The benefit of

  if (typeof define != 'undefined' && define.amd) {
     //... amd support
  }

is that all libraries can converge on the same code, making it easy to spot both for users and robots. Is there an issue with using this at all?

[guybedford]

Also, I know this project is very much designed for ember. I've made this with the assumption of compatibility, but I completely understand if the project goals don't necessary align here.

[rvagg]

Let's play the bimonthly AMD-detect switcheroo!

groan

[guybedford]

Amazing thank you so much!

[jdalton]

@guybedford I thought Volo parsed AST nodes and didn't use a regexes like parts of r.js. Were you installing it like volo add -amd bonzo, because that would pull the only tagged version, 1.0.0 which lacked AMD support, and would explain why Volo was prompting to wrap it. I tried volo add -amd https://raw.github.com/ded/bonzo/5e991c8379be8cd961ed904039dfec5a46d88f4c/bonzo.js (which is the version before this patch) and it didn't prompt to wrap.

[guybedford]

@jdalton yes I was using a volo add -amd bonzo. I can't remember if I tried this with ded/bonzo/master before the change. It sounds like you may well be right, in which case this commit can be reversed if necessary.

[jdalton]

@guybedford Ok cool. I dig the code the way it is now actually. I think escaping props for minifiers via bracket notation, like context['amd'], should be something that's moved to a build step and not in the dev code anyways.

[guybedford]

@jdalton I was under the impression advanced minification property name changes only applied to created properties not global checks like this. If there are minification issues, completely understand if it makes sense to revert though - would hate to have just made work for someone here!

[jdalton]

@guybedford No worries, no worries. Closure Compiler advanced mode will still mangle define.amd into something like define.m but really Closure Compiler advanced mode requires a bit more care and feeding than escaping a few props (as it will also mangle module.exports into something like module.n). Like I said, it's best to move that stuff to a build step anyways so it doesn't muck up your dev source.

[guybedford]

@jdalton it seems a bit strange mangling a property that isn't defined in the given code. I can understand module.exports as it is indistinguishable from a private property. I haven't wrestled much with advanced minification though.

[jdalton]

@guybedford module isn't defined in bonzo.js either. Advanced mode is advanced (super aggressive) :P

[guybedford]

@jdalton to update this discussion, the pull request is still necessary to get volo add ded/bonzo to work, without the '-amd' flag. The reason for this is that I'm using it as a project dependency, where the '-amd' flag can't be set in the volo dependency name string.

It seems the pull request is necessary, and valid as initially stated. To test try volo add ded/bonzo/v1.0.0 -amd -f and note that it attempts to convert into amd.

The -amd flag applies only to the current project, not the dependency itself. To disable the conversion, one needs to use -amdoff, which can't be used for project dependencies.

[jdalton]

@guybedford I think you typo'ed your example:

To test try volo add ded/bonzo/v1.0.0 -amd -f and note that it attempts to convert into amd.

That's because it's pulling the only tagged, v1.0.0 (~2yrs old), that lacked AMD support.

[guybedford]

@jdalton argh, ok thanks for taking the time to sort this out - you are exactly right here. So specifying the master branch to Volo explicitly is necessary for these projects, and this pull request was unnecessary.

[jdalton]

So specifying the master branch to Volo explicitly is necessary for these projects, and this pull request was unnecessary.

Naw, it still helps with AMD build optimizers like r.js or others that use regexes.

[bartsidee]

Thank you for the feedback guys. The above code seem to be working fine standalone, bundled together in ender brings some extra syntactic for me resembling jQuery. I'm using ender as a lightweight jQuery replacement, and only the build ender package is creating an issue now.

I will post an issue with details on the r.js page as it was actually working on r.js 2.06 release and now causes issues after an upgrade to r.js 2.1.4. Still I'm not sure if the issue is in r.js or if ender needs some optimizations. Ender was build using the command: ender build qwery bonzo bean traversty --sandbox