Skip to content
This repository

Add optional simple AMD support #2102

Closed
subtleGradient opened this Issue October 03, 2011 · 28 comments

8 participants

Thomas Aylott Arian Stolwijk Olmo Maldonado Aaron Newton Sean McArthur Oliver Caldwell Simon Smith Dimitar Christoff
Thomas Aylott

Come up with a simple shell script to parse out the YAML provides/requires data and then wrap each file in an AMD define.

e.g.

/*<AMD>*/define(['mootools-core/Source/Class/Class'],function(Class){/*</AMD>*/

// current code here

/*<AMD>*/});/*</AMD>*/
Thomas Aylott

Use AMD by default. Disabled by default in the online builder.

Thomas Aylott

We should also include curl.js or RequireJS in the build when AMD is enabled on the online builder. Maybe allow building in both or neither separately from AMD support.

Aaron Newton
Owner

++

Seems like we could write a little script that does this for you. No?

Aaron Newton
Owner

I'm still not clear if this definition prescribes directory locations or if mootools-core is an alias that maps to some configured location. For exampe, in the code block above you reference mootools-core/Source/Class/Class - can I configure AMD to go look in /lib/core/Source/Class/Class for that file?

Thomas Aylott

Exactly, yes. It should be relatively trivial to implement.

The tricky part would be when a single file provides multiple things.
For that you'd have to include the optional ID string in your define and then have multiple define statements.
Then you'd either have to use an AMD path config to map them all to the actual file, or else duplicate the file for each path. That would be stupid, but it'd work since requiring any one of them would provide the other and then you'd never actually load both, but that's silly.

I guess the best option would be for the script to add the define statements and generate the AMD config object for you with all the mappings.

Arian Stolwijk
Owner

:+1:

@anutron: yes. For example with require.js you can use the paths configuration.

Thomas Aylott

The important thing is that we aggressively stop anyone from bike shedding on the unimportant details.
Just add a comment that the specific paths used in your requires are definitely going to change in 2.0.
Until then it doesn't matter what exactly the require paths are. They can always be remapped with an AMD config later.

Thomas Aylott

Without an AMD config that remaps stuff, the require strings map directly to the filesystem.

A typical project would simply git clone each repo into their script folder. e.g. ...

  • /
    • index.html
    • scripts/
      • require.js
      • main.js
      • mootools-core/
      • mootools-more/
      • mootools-plugin-foo/
      • mootools-plugin-bar/

So, with this structure and no AMD config, the absolute require path 'mootools-core/Source/Class/Class' would map to Class.js from anywhere. However, from inside mootools-core itself, we would use relative require paths. So, Class.Extras.js could use the relative require path ./Class instead.

IMHO, we should NOT change the folder/filename structure of the repos before implementing the script and require paths. Sure, the require paths will be ugly until 2.0, but nobody should really care, it's just not a big deal.

Thomas Aylott

Also, since mootools-more is in a separate repo, it'll be in a separate folder and will need to use all absolute require paths to get to the stuff in mootools-core.

Sean McArthur

I'd rather if it didn't come bundled with RequireJS or curl.js, or at least give an option to leave it out. I would only ever want a simple mini require/define which basically just inserts and retrieves from a modules object.

Thomas Aylott

Multiple provides in a single file

Core.js provides Core, MooTools, Type, typeOf, instanceOf, Native.

The simplest thing to do would be to split Core.js into separate files that only provide a single thing, and then make Core.js simply require them each and then provide them all for you. That way you can require each one individually or get them all by requiring Core itself.

Alternatively we could add multiple define statements with an ID inside Core.js itself asif they had been separate files that be built into a single file using the RequireJS optimizer.
e.g.

define('mootools-core/Source/Core/Type', [...], ...)
define('mootools-core/Source/Core/typeOf', [...], ...)
define('mootools-core/Source/Core/instanceOf', [...], ...)
...
Thomas Aylott

@seanmonstar agreed. These would all be options in the builder...

  • Enable/disable the AMD packager removable blocks
  • Include curl.js
  • Include RequireJS
  • Include simple mini require/define

Ideally we should automatically include one of the AMD implementations in the UI when you manually enable AMD. That should keep people from accidentally downloading a build that won't work and then being annoyed. You'd still be able to manually disable the default or choose a different one.

Thomas Aylott

CJS+AMD is so much more beautiful
-- @cpojer

I generally agree that CJS+AMD is probably the way to go. However, that should not block supporting AMD asap. My goals are...

  1. Implement AMD asap
  2. Minimal changes necessary (code & repo structure)
  3. Allow AMD to be disabled (by default) in the online builder / Packager

We should not do anything else that would distract from, or delay, those goals

So, let's make it work first and make it pretty later. All energy on making things ideal should be focused on Milk and the MooTools.Next.

Thomas Aylott

CJS+AMD allows for littering require statements throughout the body of the module code, making it much much harder to disable with Packager removables.

Sean McArthur
Arian Stolwijk
Owner

Then why not just use the dependencies argument, it's more readable and shorter.

Thomas Aylott

If we want to use exports, that's easily doable with something like this...

/*<AMD>*/define(['exports', 'foo', 'bar'], function(exports, foo, bar){(function(){/*</AMD>*/

this.LOL = "yay!" // existing code uses `this` to apply itself to the global object

/*<AMD>*/}).call(exports)}})/*</AMD>*/
Thomas Aylott

Also, we need to be sure we're running all our tests against the AMD version of the code and with all the AMD stuff stripped out.

Thomas Aylott

@seanmonstar @arian @cpojer It doesn't matter exactly what the boilerplate is, it only matters how much time and how many code changes are required to make it actually function.
We can debate the details in the thread of a pull request once we have one to talk about.

Arian Stolwijk
Owner

So for now just use what works (standard AMD :D)

Arian Stolwijk
Owner

Little update, see https://github.com/arian/mootools-core/tree/1.5amd / arian@1.5wip...1.5amd

Todo: test it more, use the requirejs optimizer, with !amd compat block (extending stuff into the global object), etc.

Thomas Aylott

woah, looking nice!
I'm very excited to see this moving forward

Oliver Caldwell

Good job I searched for this first rather than going straight ahead with it. I was planning on writing a YAML to AMD converter mainly built for MooTools. It would recurse over a directory reading all files and parsing their provides and requires. Then it would map the requires to the files that provide them and finally write them back.

The result would either be an identical directory structure or a completely flat one. The new copy would have every source file wrapped in the correct define call. It would be a separate project to MooTools so people could convert any version they want into AMD.

I just wondered what your thoughts were on this? Because I would love to write it.

Thomas Aylott
Owner

Go nuts.
It'd be nice to convert to CommonJS instead. There are already a million things that can convert that to AMD afterward.

Simon Smith

Are there still plans to make 1.x AMD compat?

Arian Stolwijk
Owner
arian commented April 17, 2012

Yes, within a few weeks I'll try to finish https://github.com/arian/mootools-core/tree/1.5cjs. The possible build options will be:

Dimitar Christoff

@arian ++ go go go!

Thomas Aylott subtleGradient referenced this issue in mootools/mootools-more May 22, 2013
Closed

Simple AMD #1043

Olmo Maldonado ibolmo closed this March 03, 2014
Olmo Maldonado
Owner

Likely not going to happen anytime soon™.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.