jcoglan / sylvester

Vector, matrix and geometry math JavaScript
http://sylvester.jcoglan.com
1.15k stars 124 forks source link

Add AMD support #8

Open feklee opened 11 years ago

feklee commented 11 years ago

Please consider adding support for AMD, the Asynchronous Module Definition. The AMD plus web pattern seems suitable and it is easy to implement:

(function (root, factory) {
    if (typeof define === 'function' && define.amd) {
        // AMD. Register as an anonymous module.
        define(factory);
    } else {
        // Browser globals
        root.amdWeb = factory(root.b);
    }
}(this, function () {

    // Current Sylvester code

    return {
        Sylvester: Sylvester,
        Vector: Vector,
        Matrix: Matrix,
        Line: Line,
        Plane: Plane,
        $V: $V,
        $M: $M,
        $L: $L,
        $P: $P
    }
}));
jcoglan commented 11 years ago

Is this required to make the library work on the environment you're using? Could it not be glued in using, for example, Require.js shim config?

feklee commented 11 years ago

The problem for me is pollution of the global namespace (window). I am writing an API (to a widget) which includes Sylvester. I have no control over the JavaScript on pages that include the API's JavaScript interface. So, with the stock Sylvester code, there could be conflicts. AMD is a great way to keep code organized in several files without polluting the global namespace.

With the above code, loading Sylvester is just plug and play. There are further Universal Module Definition patterns to support more module loaders simultaneously.

jcoglan commented 11 years ago

Is this a pressing problem, rather than a nice-to-have? Would it be improved by, say, namespacing all the classes so you'd have Sylvester.Matrix rather than Matrix?

My problem with all these module systems is that they require boilerplate adding to all your source files, which having every file in a project deal with all the ways JS code can be loaded and making sure the tests still run on all the target platforms is a lot of work, especially when compared to how much of a problem it actually is to give the project a namespace.

Are there platforms you're targeting where AMD is the sole means of loading code?

feklee commented 11 years ago

Is this a pressing problem, rather than a nice-to-have?

Not polluting the global namespace is essential for my project. But of course, wrapping Sylvester into an AMD module is not that big of a deal. I just did it myself.

Would it be improved by, say, namespacing all the classes so you'd have Sylvester.Matrix rather than Matrix?

That would still pollute the global namespace. Furthermore, namespacing all objects into Sylvester would break existing code (not mine) depending on Sylvester.

My problem with all these module systems is that they require boilerplate adding to all your source files [...]

Just to make sure that there isn't a misunderstanding: The above AMD-plus-web pattern does not break any existing code depending on Sylvester. If AMD is not available (!define.amd), then the Sylvester library works as before, with Sylvester, Vector, etc. in the global namespace.

[...] making sure the tests still run on all the target platforms is a lot of work [...]

If I understand you correctly, then with Sylvester supporting AMD, you would want to make sure that Sylvester still passes all the tests when included via AMD.

Are there platforms you're targeting where AMD is the sole means of loading code?

No. However, my API's JavaScript code (which utilizes Sylvester) should not pollute the global namespace. People may load it as AMD (planned) or without.

jcoglan commented 11 years ago

Is this a pressing problem, rather than a nice-to-have?

Not polluting the global namespace is essential for my project. But of course, wrapping Sylvester into an AMD module is not that big of a deal. I just did it myself.

There's a few questions I have about this:

The second question is important: by using AMD, you're simply replacing bindings to global JS identifiers with bindings to global module names, which amounts to the same thing. Any code in the runtime can clobber the API of a module by requiring it and making changes to it, and can even clobber the module out of existence completely, either by redefining it by name or by altering its load path using, say, requirejs.config(). It does not (and in fact is not designed to) eliminate global shared state at all. Global shared state is necessary for the AMD module registry to function.

Second, it's worth noting that even if Sylvester did expose an API via define(), we would not be able to remove global names. If a library that depends on Sylvester is included in an environment where AMD is being used, but is not itself AMD-aware, it still needs to refer to Sylvester using global names. Look at how jQuery does this, and look at why Underscore removed AMD support.

My problem with all these module systems is that they require boilerplate adding to all your source files [...]

Just to make sure that there isn't a misunderstanding: The above AMD plus web pattern does not break any existing code depending on Sylvester. If AMD is not available (!define.amd), then the Sylvester library works as before, with Sylvester, Vector, etc. in the global namespace.

It would, for the reason given above. This only works if you have buy-in from all the libraries you're using, and even if you have that you haven't got rid of global state. It seems wasteful to me to have every library out there provide explicit support for whatever module loader people want to use, when said module loaders already provide ways to load code without modifying the source.

You can easily use AMD to load Sylvester as it stands, using Require.js shim config: http://requirejs.org/docs/api.html#config-shim. This lets the user choose how to load something, without imposing a large maintenance burden on library authors.

[...] making sure the tests still run on all the target platforms is a lot of work [...]

If I understand you correctly, then with Sylvester supporting AMD, you would want to make sure that Sylvester still passes all the tests when included via AMD.

Right, which as I've said is a lot more boilerplate and coupling for no benefit that the user cannot already extract by themselves.

Are there platforms you're targeting where AMD is the sole means of loading code?

No. However, my API's JavaScript code (which utilizes Sylvester) should not pollute the global namespace. People may load it with AMD or without.

Since there are other options for loading code and exposing the API, both using Require.js and other loaders, and for the arguments given above, I'm included not to go ahead with this request.

jrburke commented 11 years ago

Some responses to your questions:

  • Why is it essential not to pollute the global namespace? Are you actually getting naming conflicts from the libraries you're using?
  • How is using AMD preferable to using global names?

Any new module format is not going to go with globals. That is not the path commonjs, amd or the ES harmony proposals have taken. So any module format that ends up "winning" will encourage developers to not use globals.

However, as that transition to modular, non-global based code happens, not everyone will be able to use a library in that fashion, at least not right away. So libraries will still export a global so that they can be used in those older projects. It is an effort to ease transition. This is just how it works on the web. It is rare that anything this large can be switched on overnight. This is what underscore needed to do too, but I believe Jeremy just did not want to think about it more, and wants to wait and see if the ES group can just get some modules in the language natively.

So, that is a stance to take: 'I do not think AMD has enough to qualify as a module standard yet'. However, nothing else in the browser space comes close. AMD has multiple implementations and the most adoption of a browser-based format. It is how standards are made -- multiple implementations, with some coordination across the implementations, get real world use. The only thing that will probably kill it off is if ECMAScript modules. But those harmony proposals are not complete. In their current state, they will not kill off AMD-based loaders, and I expect those harmony proposals to be modified further based on the real world use of AMD.

However, most people making larger web apps are not going to choose "I will use browser globals, and I will try to work out the right load order by manually authoring script tags".

As for trading a global string name for a global string identifier: there is more to it than that. By using anonymous modules, it means the consumer of your module gets to name the module, the module does not name itself, as is the case with browser globals.

This allows people to swap in different providers of a module ID without worrying about needing to modify source (think using zepto instead of jquery for the 'jquery' dependency ID), and it allows the end user to load two different versions of the library, just by naming the file differently, using loader config, or by loading the versions in two different 'module containers', like the requirejs multiversion support does. No need for noConflict APIs, which are brittle anyway with async script loading in today's browsers.

While requirejs does not limit access to globals, an AMD loader could be constructed to do so, by loading each module in an iframe, or doing some tricks with XHR + eval. The AMD module format does not need to change for that to happen. However, the runtime resource cost is usually not worth it, and there are times that some modules need globals -- just to be productive since some existing JS code is not module-aware yet, and also because the browser does not support grabbing important objects like document and window via a module API yet.

Hopefully that helps answer some of your questions, even if you want to wait and see what happens over time.

feklee commented 11 years ago

Why is it essential not to pollute the global namespace? Are you actually getting naming conflicts from the libraries you're using?

No, worse: I am creating a hosted library (API), and I don't know in which environments it will be used.

How is using AMD preferable to using global names?

That depends on use-case. My library should at max. introduce one global. All the libraries (incl. Sylvester) that it depends on, should be hidden. The user should just not have to think about them.

The AMD-plus-web pattern that I posted works automagically:

Any code in the runtime can clobber the API of a module [...] or by altering its load path using, say, requirejs.config().

Admittedly, I don't entirely understand what you are trying to tell me. But my library will be totally unaffected by requirejs.config(). The reason: The release version of the library will be one monolithic file.

You can easily use AMD to load Sylvester as it stands, using Require.js shim config [...]

That will still introduce the globals.

[...] I'm included not to go ahead with this request.

Alright. I am already wrapping Sylvester myself into an AMD module.

petermichaux commented 11 years ago

Module system boilerplate is a cancer spreading through the JavaScript community. The boilerplate grows larger and more complex as time goes by and there are more loader systems to support. Using a single global variable like Sylvester is simple and does the job of avoiding naming conflicts.

feklee commented 11 years ago

Using a single global variable like Sylvester is simple and does the job of avoiding naming conflicts.

In my case, it doesn't.

Raynos commented 11 years ago

@petermichaux it does in the naive view where one develops a module which itself does not have dependencies. Which is clearly false maria has a whole slur of dependencies each of which define their global variables.

Look at npm. look at dependency graphs that go down deep. Look at all the authors working together building small modules that do one thing well so that each library doesn't have to reinvent and bundle every wheel of it's own.

A single global variable does absolutely nothing to avoid naming conflicts, the fact it does so is simply an act of programming by coincidence.

The only thing that can avoid naming conflicts is guarantee that a name is unique. Look at npm again for an example, each module name in npm can only be taken by one library. Each require call in node's style commonJS can only call a unique npm name or a unique file name.

jcoglan commented 11 years ago

@jrburke With the caveat that I don't want this to turn into a debate about AMD/modules per se, only insofar as they affect Sylvester, I still have some questions. Please forgive me if I appear slow on the uptake.

Any new module format is not going to go with globals. That is not the path commonjs, amd or the ES harmony proposals have taken. So any module format that ends up "winning" will encourage developers to not use globals.

Can you explain how AMD discourages globals? AMD module names are global just as much as JS variable names are, and modules can easily modified or completely replaced, incurring exactly the same problems are global variables i.e. shared mutable state. There is nothing to stop modules' names colliding since there is no central repository (like npm).

For example, say I write:

require(['./foo', './bar'], function(foo) {
  foo.run();
});

Where foo.js contains:


define('foo', ['./qux'], function(qux) {
  return {
    run: function() {
      var p = document.getElementById('result');
      p.innerHTML = qux.getMessage();
    }
  };
});

qux.js contains:


define(function() {
  return {
    getMessage: function() {
      return 'Hello world!';
    }
  };
});

and bar.js contains:


define('bar', function() {
  return {};
});

define('qux', function() {
  return {
    getMessage: function() {
      return 'U HAV BEAN PWNED!!!';
    }
  };
});

then bar.js clobbers the definition of the qux module, even though qux is anonymous and referred to be a relative path. Similarly, modules can load others and change their interfaces, or they can use requirejs.config() to supplant paths to modules to replace modules.

This has pros and cons, but they are (it seems to me) the same pros and cons displayed by global variables.

However, as that transition to modular, non-global based code happens, not everyone will be able to use a library in that fashion, at least not right away. So libraries will still export a global so that they can be used in those older projects.

Agreed -- this is why jQuery does this, and why I won't be able to get rid of globals for @feklee.

So, that is a stance to take: 'I do not think AMD has enough to qualify as a module standard yet'

I'd like to clarify that this is not the stance I'm taking. I like AMD's design, it's pretty much the only sane thing you can do in the browser. My point here is simply that people can use it without modifying the source of the libraries they use, and that it does not (at least at present) allow us to remove global variables from libraries without breaking backward compatibility.

I think the pattern of having libraries decoupled from code loaders is the only maintainable thing to do at present. This does not reduce the utility of module loaders, and keeps library source files free of coupling to multiple different loading strategies, which incurs a high maintenance cost.

However, most people making larger web apps are not going to choose "I will use browser globals, and I will try to work out the right load order by manually authoring script tags".

This seems like a straw man to me. It's perfectly possible to minimise implicit knowledge about dependencies while keeping this information separate from the source code, while keeping source loadable in many different environments. This what JS.Packages does, as explained on my blog. (This is not to plug the library, merely to demonstrate the loader and the source can be decoupled, and that that is my preferred strategy.)

As for trading a global string name for a global string identifier: there is more to it than that. By using anonymous modules, it means the consumer of your module gets to name the module, the module does not name itself, as is the case with browser globals.

If the end user names the module (e.g. by choosing where on the filesystem to place it), how does this promote reusable dependencies? If we do not have a stable name by which to refer to a dependency, what do we do instead? Does this rely on the user using requirejs.config() to specify the path at which to find each module? How does it avoid (if at all) a module name being clobbered in the same way that browser globals can be?

jcoglan commented 11 years ago

@feklee In light of my previous comment:

The AMD-plus-web pattern that I posted works automagically:

  • Sylvester included with AMD -> no global introduced
  • AMD not detected -> standard set of Sylvester globals introduced

It does not work 'automagically', I need to do a substantial amount of work by coupling Sylvester's source and tests to AMD, CommonJS, or globals as appropriate.

And, as explained by @jrburke and me, AMD does not allow us to remove globals, at least with the present state of the module ecosystem.

jcoglan commented 11 years ago

@Raynos said:

The only thing that can avoid naming conflicts is guarantee that a name is unique. Look at npm again for an example, each module name in npm can only be taken by one library. Each require call in node's style commonJS can only call a unique npm name or a unique file name.

I agree with this: if we want portable dependency references, we still need globally stable names for them and a central repository of such names to enforce uniqueness.

CommonJS makes it harder to clobber modules as well: you can still require them and modify the exports object from outside (unless the author uses Object.freeze()) but you cannot entirely replace a module without writing to the filesystem. AMD makes it easy to clobber modules entirely within the JS runtime, both through define() and through changing path mappings.

Not that this is an argument for favouring CommonJS: it won't work in the browser. I'm just pointing out places where I don't quite buy the benefits people are touting for AMD.

feklee commented 11 years ago

@jrburke, please correct me if I'm wrong.

AMD module names are global just as much as JS variable names are, and modules can easily modified or completely replaced, incurring exactly the same problems are global variables i.e. shared mutable state. There is nothing to stop modules' names colliding since there is no central repository (like npm).

It is discouraged to name modules, and "there should only be one module definition per file on disk.".

For example, say I write: [...]

I'll have a look at the example when you rewrite it following the above rules.

feklee commented 11 years ago

And, as explained by @jrburke and me,

Where does @jrburke say that?

AMD does not allow us to remove globals, at least with the present state of the module ecosystem.

AMD modules don't expose any globals, unless the author willingly makes statements such as window.myGlobal = 5.

jcoglan commented 11 years ago

Where does @jrburke say that?

"However, as that transition to modular, non-global based code happens, not everyone will be able to use a library in that fashion, at least not right away. So libraries will still export a global so that they can be used in those older projects." -- https://github.com/jcoglan/sylvester/issues/8#issuecomment-7229806

Libraries still have to expose globals so they can be used by non-AMD-aware clients.

feklee commented 11 years ago

Libraries still have to expose globals so they can be used by non-AMD-aware clients.

I don't dispute that. In fact, that's what the AMD-plus-web pattern is for.

kalleth commented 11 years ago

So I've been following along (barely) with what's being discussed, and one thing I don't understand; if it's so easy to wrap Sylvester within an AMD definition, what's the argument for asking a library author to add that to his library, rather than when someone is working within that environment, wrapping it himself?

jcoglan commented 11 years ago

Libraries still have to expose globals so they can be used by non-AMD-aware clients.

I don't dispute that. In fact, that's what the AMD-plus-web pattern is for.

That isn't what this pattern is for. If you use this pattern, then a non-AMD aware library that's loaded in an AMD-using environment will not be able to refer to its dependencies.

jcoglan commented 11 years ago

So I can properly answer @kalleth: is there an AMD implementation that runs in all these environments? This is what JS.Packages supports, and is where I currently need to test Sylvester to ensure maximal portability.

feklee commented 11 years ago

If you use this pattern, then a non-AMD aware library that's loaded in an AMD-using environment will not be able to refer to its dependencies.

Sorry, I don't get that. When you have a web-app that makes use of AMD (e.g. by loading require.js), and you load a non-AMD aware library (e.g. backbone.js), then you have to take care of loading dependencies yourself (such as jquery.js and underscore.js). Yes, of course. How else could it be.

feklee commented 11 years ago

what's the argument for asking a library author to add that to his library,

Comfort.

rather than [...] wrapping it himself?

That's what I'm doing - read above.

kalleth commented 11 years ago

@feklee I know that's what you're doing, I'm saying why ask a library author to do so for you when it may not be appropriate for all environments the library is intended for. 'Comfort' makes no sense to me.

I don't particularly want to take sides or pollute the argument further though; I don't have much to offer refs the substantive portions of @jcoglan's objections, don't know enough :)

jcoglan commented 11 years ago

When you have a web-app that makes use of AMD (e.g. by loading require.js), and you load a non-AMD aware library (e.g. backbone.js), then you have to take care of loading dependencies yourself (such as jquery.js and underscore.js). Yes, of course. How else could it be.

My point is that executing sylvester.js has to result in the exposure of an API that both AMD and non-AMD code can consume. If the script does not export any globals, how can non-AMD code refer to its API?

jcoglan commented 11 years ago

Can I pre-emptively interject, since this is getting a lot of attention: I absolutely do not want to make this about berating the OP, or authors of module libraries. I'm trying to understand the situation so I can make an informed decision about where responsibility should lie in our respective codebases. Admittedly this may be annoying the hell out of some people, and I apologize for my ignorance.

Please stay polite and on-topic (this is not about AMD in general, but about its applicability to a single platform-agnostic portable library).

Thanks everyone for your input so far.

feklee commented 11 years ago

My point is that executing sylvester.js has to result in the exposure of an API that both AMD and non-AMD code can consume. If the script does not export any globals, how can non-AMD code refer to its API?

There could be a misunderstanding. Let's for a moment assume that Sylvester had the AMD-plus-web pattern implemented. Then:

jcoglan commented 11 years ago

This is the case I'm concerned about:

Loading sylvester.js without AMD does not work. Then the Sylvester API is unreachable.

If you're using an AMD loader, and a library does not expose an API other than via AMD in that environment, then you cannot use anything that does not yet understand AMD that needs to talk to the library. This is why jQuery exposes a global regardless of whether AMD is in use of not.

feklee commented 11 years ago

[...] then you cannot use anything that does not yet understand AMD that needs to talk to the library.

True.

This is why jQuery exposes a global regardless of whether AMD is in use of not.

I am aware of that.

jrburke commented 11 years ago

@jcoglan sorry, this discussion seems to be getting a bit unwieldy. If you would rather talk offline or even in some other channel, feel free to point me to it. To respond to some of your questions:

Can you explain how AMD discourages globals? AMD module names are global just as much as JS variable names are, and modules can easily modified or completely replaced, incurring exactly the same problems are global variables i.e. shared mutable state. There is nothing to stop modules' names colliding since there is no central repository (like npm).

CommonJS makes it harder to clobber modules as well: you can still require them and modify the exports object from outside (unless the author uses Object.freeze()) but you cannot entirely replace a module without writing to the filesystem. AMD makes it easy to clobber modules entirely within the JS runtime, both through define() and through changing path mappings.

You could construct an AMD loader that freezes the returned module value, just as you could in CommonJS. The point is that by having a clear module API (either setting things on an 'exports' object or returning a value from an AMD factory function), it clearly states the export for a module. That is very difficult to do with window globals.

So while an AMD loader could prevent module modifications and even force a "single module define() per file", most web apps do not need these restrictions. But the AMD module API allows for it, it does not have to change. All of the benefits of CommonJS in this respect apply to AMD -- you can take a CommonJS module and wrap it with define(function(require, exports) {}) to make is an AMD module.

Similarly, a developer could hide requirejs.config() once they set up the top level config they want. This would be similar to setting security policies on some AMD loader that provided the stricter freezing and such.

I agree with this: if we want portable dependency references, we still need globally stable names for them and a central repository of such names to enforce uniqueness.

I do not believe a central repository will not work, it is ultimately not scalable, not fitting with decentralized web design. The most we can hope for is commonly understood interface names. If someone asks for a 'jquery' dependency, that it generally conforms to the API jQuery provides. Some people on mobile want to use Zepto on mobile instead of jQuery, but still use code that uses 'jquery' as the dependency name.

I think the pattern of having libraries decoupled from code loaders is the only maintainable thing to do at present. This does not reduce the utility of module loaders, and keeps library source files free of coupling to multiple different loading strategies, which incurs a high maintenance cost.

I am not sure I follow this. If I read it correctly, I believe AMD fits this description: libraries use define() and string names for dependencies. How those string dependencies are resolved, what paths they map to, are part of a loader API.

Maybe what you are objecting to is that define() needs to be used in a library. Any eventual module format in JS will require changes to the library for it to work, at least based on the current module systems in use in JS: CommonJS, AMD, ES harmony proposals. Even YUI.

This seems like a straw man to me. It's perfectly possible to minimise implicit knowledge about dependencies while keeping this information separate from the source code, while keeping source loadable in many different environments. This what JS.Packages does, as explained on my blog. (This is not to plug the library, merely to demonstrate the loader and the source can be decoupled, and that that is my preferred strategy.)

A system that requires module to name itself is unlikely to gain traction. But the story definitely is not done. So who knows for sure. The odds are not good though based on today's state. Dojo had a system where modules named themselves, but found anonymous modules and the other AMD benefits compelling enough to move away from it.

There still needs to be a wrapped format for bundled modules -- some folks like to load some scripts from the CDN, and if a bundled module needs that dependency before it executes, there will be a wrapped format, like define(). Maybe you hope to do all external loading up front, but that delays some code execution longer that it needs to be -- I would not want all my code to have to wait for say, a facebook like script to load. Once that wrapped format is added in, it likely starts to look a lot like define().

But now I'm speculating on how you might wish to solve some other issues with modules and module loading/bundling/dynamic loading. Probably best to stop there.

I do agree that there is a separation of dependency info: for runtime, the module just needs to reference a dependency like 'jquery', but what 'jquery' got installed on disk may be specified as part of a package.json metadata.

As far as platform support, requirejs runs in all browsers, and via r.js, in rhino and node. Other environments have been made to work by overriding requirejs.load() to do the right IO operation for a particular environment. Firebug used it for use in Gecko, it was adapted by someone else for use in xpcshell, and I will be adding WSH because some .NET folks want to use the optimizer without running Node.

Thanks for keeping the thread focused an informative. Hopefully the above helps if you decide to consider AMD later.

kalleth commented 11 years ago

@jrburke please keep discussion here if poss - I'm learning loads reading through yours and @jcoglan s responses!

Raynos commented 11 years ago

@jcoglan

CommonJS makes it harder to clobber modules as well: you can still require them and modify the exports object from outside (unless the author uses Object.freeze())

This is a feature. It's called monkey patching. You can disable monkey patching on an object by object basis using Object.freeze

Not that this is an argument for favouring CommonJS: it won't work in the browser.

It does work. See browserify. See wreq.

jcoglan commented 11 years ago

CommonJS makes it harder to clobber modules as well: you can still require them and modify the exports object from outside (unless the author uses Object.freeze())

This is a feature. It's called monkey patching. You can disable monkey patching on an object by object basis using Object.freeze

Yes, I get that -- it's what I meant when I said that modules and globals have the same pros and cons. @jrburke's point that AMD allows Object.freeze() to be transparently introduced as a valid one, though.

Not that this is an argument for favouring CommonJS: it won't work in the browser.

It does work. See browserify. See wreq.

Can I qualify my assertion with, "won't work without a compiler step". This is not a trivial point, it's a genuine barrier to adoption. I've tried building libraries with Browserify and it makes it really hard to export an API correctly. It also introduces an unacceptable boilerplate into the build artefacts, considering this is code that will be read by strangers, rather than simply deployed as part of an app and never read in its deployed format. Browserify works much better for building apps than libraries.

Wreq's a neat demo, and very clever, but completely unsuitable for production.

Raynos commented 11 years ago

@jcoglan

@jrburke's point that AMD allows Object.freeze() to be transparently introduced

That would break so much code

won't work without a compiler step

Just have the server compile it on the fly rather then serve static JavaScript. This get's rid of almost all your worries with the compiler.

I've tried building libraries with Browserify and it makes it really hard to export an API correctly.

The easiest thing is to just export the API in a commonJS compliant fashion and tell your library users commonJS is the future.

Wreq's a neat demo, and very clever, but completely unsuitable for production.

It's not production ready but it proofs there's absolutely nothing stopping commonJS apart from most of the community is wasting their time playing with their AMD toys

tbranyen commented 11 years ago

@Raynos I find it ironic you advocate wreq and then call AMD a toy.

jcoglan commented 11 years ago

Guys: on topic, please. I am not having this expand into a 'whose module library is best' debate.

I have a bunch of questions I'm going to ask @jrburke via email so I can figure out what I want to do.

tbranyen commented 11 years ago

@jcoglan Not sure why you don't just stick with the shim configuration (just document it). Its based off the work I did with https://github.com/tbranyen/use.js/ and my intention was if you wanted to load a non-amd compatible script (and remove globals) you could simply provide a function like this:

shim: {
  "libs/sylvester": {
    exports: function() {
      var global = this.Sylvester;
      delete this.Sylvester;

      return global;
    }
  }
}

IMO you should start with an AMD definition, but not convert an existing project, especially since its not necessary.

jrburke commented 11 years ago

@tbranyen: Sylvester exports a few globals, so I would probably not use that function syntax, or it would need to be expanded. It makes it harder to use the IE load error detection too. Also, the shim config mentioned above should use exports: not export.

So if it was documented, I would probably just point people to "you can use shim config and look for the global Sylvester if the developer wants to catch load errors in IE.

Raynos commented 11 years ago

@tbranyen I never said use wreq. I wouldn't.

tbranyen commented 11 years ago

@jrburke Fixed the typo. How many globals there are is irrelevant to the concept. Just attach them to an "exports" object and prune them off the global. The desire to do that is edge case; I'm sure the developer doing this won't mind the extra few lines of explicit code.

Okay so the IE load errors is a bummer, but we can live with that... meaning it's not a perfect solution, but it's also an incredibly low barrier to entry.

ewogreg commented 11 years ago

@jcoglan

This is the case I'm concerned about: Loading sylvester.js without AMD does not work. Then the Sylvester API is unreachable. If you're using an AMD loader, and a library does not expose an API other than via AMD in that environment, then you cannot use anything that does not yet understand AMD that needs to talk to the library. This is why jQuery exposes a global regardless of whether AMD is in use of not.

Agreed. Now, just my 2 cents on the subject :

The main issue with the AMD-plus-web pattern is that if you implement it, any library relying on Sylvester might break in an environment where a "define" function has been declared (say, requirejs). I can't argue with that, and that is probably why you should still export a global variable. However, if you don't add some kind of AMD support in parallel, how can we expect for the libraries depending on Sylvester to ever evolve ?

The pattern @feklee suggested might be too much for now, but you can definitely add some way to export for AMD-aware developers without breaking backward compatibility. This would be a win-win, and might allow for others libraries relying on Sylvester to do the same.

feklee commented 11 years ago

Small correction:

[...] where a "define" function has been declared [...]

[...] where define.amd is true [...]

OT, and without further thinking: It could be a cool idea to have an officially supported AMD option, such as define.amd.globals, controlling whether the user wants compatibility globals or not.

Raynos commented 11 years ago

@ewogreg

However, if you don't add some kind of AMD support in parallel, how can we expect for the libraries depending on Sylvester to ever evolve ?

Sylvester already supports commonJS so other libraries dependending on Sylvester can simple be commonJS compliant

fiznool commented 11 years ago

@feklee A simple way that AMD support was added to the Flotr2 library was by including a separate AMD build step to wrap up the built library file in the AMD-plus-web syntax. The general idea is to split the AMD-plus-web definition into two - pre and post - and sandwich the built library in between.

I did this on my fork so that I could update the core library and build it myself whenever the upstream was updated. It's an alternative to using shim, not ideal but might be a short-term solution.

// pre.js
(function (root, factory) {
    if (typeof define === 'function' && define.amd) {
        // AMD. Register as an anonymous module.
        define(factory);
    } else {
        // Browser globals
        root.amdWeb = factory(root.b);
    }
}(this, function () {

    // Current Sylvester code

Use cat or something else to insert the built Sylvester library here

// post.js
return {
        Sylvester: Sylvester,
        Vector: Vector,
        Matrix: Matrix,
        Line: Line,
        Plane: Plane,
        $V: $V,
        $M: $M,
        $L: $L,
        $P: $P
    }
}));

The pre, post and build steps were eventually merged into Flotr2.

feklee commented 11 years ago

[...] a separate AMD build step [...]

@fiznool, thanks for the suggestion! In fact I have thought about something like this. However, for now, manually wrapping seems sufficient and simpler. I wrap jQuery and Underscore.js in similar fashion.