requirejs/require-cs

Cross domain

Opened this issue · 15 comments

guybedford commented

Would you consider using the same approach here as the text plugin?

I really think this is the way to go actually, although ideally it should apply to template and css plugins as well. Happy to get involved in this area as it is on my critical path right now, so let me know how I can go about it in a way that is useful here as well.

guybedford commented

I've made a fork here https://github.com/guybedford/require-coffee which includes cross domain support like the text plugin. To exclude CoffeeScript in this case, I had to turn it into a folder-based plugin as discussed previously.

I'm likely to provide something similar for the require-less plugin and ideally for template plugins as well.

jrburke commented

I wonder if it is best to convert this plugin to just depend on the text plugin. That way it avoids a bunch of duplication. What do you think?

guybedford commented

Yes I strongly feel that there is enough of a common set of requirements for a transpiler to provide easier way to create transpilers.

This would also help improve the quality of template plugins for example, and make it easier for others to provide new transpilers.

I do like the idea of having some base generator that can be used by everything else, and it makes sense to keep this out of the requirejs core initially. The text plugin is therefore the most suitable candidate, although it could also go by another name, and the text plugin could inherit from it as well.

After a little consideration, here is a sample API I would find extremely useful to cover most of my plugins:

cs.js:

  define(['text'], function() {
    return text.createLoader('.coffee', function(source, load) {
        require(['coffeescript'], function(CoffeeScript) {
          load.fromText(CoffeeScript.compile(source));
        });
      }
    });
  });

Note:

  • The compiler is loaded dynamically so that the compiler is excluded by default in builds.
  • The 'useXhr' settings for the text plugin would then apply to all loaders created by the createLoader function.
  • The writeFile method would also be supported by all loaders as well as provided by the createLoader function.

I'd be happy to prototype a text plugin with a function like the above. Let me know your thoughts.

guybedford commented

The other approach to cross domain is to enable CORS. Ideally if the text plugin can create other loaders, the core AJAX implementation would have a configuration mode to allow choosing between 'precompiled' and 'cors'. I think that would give a lot of flexibility for this sort of thing.

Quite keen to get going on the development work, may well just start tinkering with a fork text plugin in the mean time, but if there is any way that I can adapt to fit better with things your side let me know.

jrburke commented

Maybe that transpiler helper is just some other plugin that happens to duplicate functionality from the text plugin, but is designed specifically for transpilers, which seems fine. However, I would keep it separate from a text plugin, which has uses beyond language transpilers.

guybedford commented

Ok, do you think the API makes sense? It would be great to grow the ecosystem of template plugins by making this process simpler, so it would be good to know if this seems like its on the right track. If not, or if you want to bring the features into the text plugin in a different way, please let me know.

jrburke commented

For the API, I don't think you can dynamically load coffeescript and have that work in a build -- for it to run in a build, only define()'d dependencies are brought into full form during a build.

I would likely not do 'text.createLoader', as mentioned above, but put that on whatever the new transpiler plugin is, so 'transpiler.creatLoader()', that sort of thing.

I think it will take a bit of experimentation though, so best to try something to see how it holds together.

guybedford commented

Ok thanks.

Yes one doesn't want coffeescript in the build - hence why it is taken out of the 'define' statement. It allows a no-config build.

Just to get my thinking right on how this could apply to the require-cs plugin - I know you probably don't want this module to have a dependency, and if it does, having a dependency on the text plugin feels easier than "transpiler", which could be a bit heavier and may be a harder thing to commit to this plugin.

I suppose these issues ultimately need to be worked out in the package management space though.

What I will do then is create a "transpiler" base plugin, and update my "require-cs" fork to be based on it. Whether it gets pulled or not then is obviously up to you.

Will post back here when I have something up soon.

guybedford commented

I've put together a first version here - https://github.com/guybedford/loader.

It includes support for both CORS and having a '.js' extension.

See the 'test' folder for the examples. Specifically run test.html and test-build.html.

With the helper, the require-cs module becomes:

cs.js:

define(['loader'], function(loader) {
  if (loader)
    return loader('coffee', function(name, source, req, callback, errback) {
      require(['coffee-script'], function(CoffeeScript) {
        callback(CoffeeScript.compile(source));
      });
    });
  else
    return { pluginBuilder: './cs-builder' };
});

cs-builder.js:

define(['loader', 'coffee-script'], function(loader, CoffeeScript) {
  return loader('coffee', function(name, source, req, load, config) {
    load(CoffeeScript.compile(source));
  });
});

We need to separate the plugin builder when using a loader base, as otherwise the loaded dependency of loader is undefined on the server.

The added benefit of the dynamic require is that by default coffee-script will be excluded from any build. This is still pending issue requirejs/r.js#289 though.

Please let me know if there is anything I can do to make it more accessible, in the mean time will continue with a fork of require-cs.

jrburke commented

Great to see this moving along. Some notes:

We need to separate the plugin builder when using a loader base, as otherwise the loaded dependency of loader is undefined on the server.

Do you mean that coffee-script is undefined? I expect loader to be defined, otherwise there is a bigger problem.

The added benefit of the dynamic require is that by default coffee-script will be excluded from any build. This is still pending issue requirejs/r.js#289 though.

I still favor a plugin that does not need a pluginBuilder, as it is just easier to pass around and use, and would rather just encourage people do an exclude: ['cs'] for the build if they do not want to include coffee-script.js, cs.js and loader.js in the built file. Then set stubModules: ['cs'] so that what ends up in the built file is just a small define for 'cs' if it is not needed after a build.

That also assumes the loader plugin does not implement a "normalize" method, but looks like your base loader plugin does. I would try to implement it without a normalize method.

guybedford commented

Yes sorry I should mean that coffee-script is undefined! The pluginBuilder works here because of the parsing mechanism.

Yeah I don't think we've been able to agree on this pluginBuilder issue actually. It's come up a few times, and I still feel quite strongly that:

  1. It should be possible to use and build a module without needing to provide any configuration.
  2. stubModules is not a good idea because dynamic requires in production are stopped, which is a major benefit of RequireJS over any other loading system.

The approach I've been taking is merely as a solution to the above, but I admit that the pluginBuilder approach is by no means an elegant one, at the same time the idea of client / server code separation does make the most natural sense though.

Any feedback on 1 and 2 appreciated. Perhaps it is worth getting feedback from wider sources?

jrburke commented

It seems like you want to optimize for the case that the user has built in some cs modules, but will dynamically load some later, but you want to delay loading of coffee-script.js until the first dynamic require happens.

If that is how people use the coffeescript plugin, that is great. I started out avoiding plugins that need more than one file, as it requires more instructions to the end user about where to place things, but since there is already a dependency on coffee-script.js maybe it is not too onerous to have a pluginBuilder file.

I definitely think it is worth opening up to other coffeescript+requirejs users, but I'm also fine if you go with the pluginBuilder route and try it out, collect feedback on how it goes. It is easy enough to switch up if it does not work out, and at this point you have much more experience with a transpiler plugin than I do.

guybedford commented

Most likely the later module calls will be for modules that have been precompiled (.coffee.js), so one wouldn't load the full compiler as well in this case. But I like that it would still be possible to type in the console and have full equivalence between the development and production environments.

That said, it would be much simpler leaving out the plugin builder. When introducing the loader, I would introduce the simple format first (without a plugin builder), and the pluginbuilder second as a (less elegant) convenience. But I do think that the convenience is the most important for the adoption.

I think the main benefit of a generic 'transpiler' plugin is in widening the transpilers we have and ensuring they are of high quality, so there should be a focus on best practise here. This could potentially be used as a base for everything from require-css to handlebars to es6 shims.

The Require-CS project is a great place to test it out. I will do some further testing and then would be keen to integrate. Let me know if you are happy in the current form or have any further feedback. Shall I push to master or a separate development branch?

Also to make installation easier, how would you feel about adding a component.json as well as the volo properties?

jrburke commented

That said, it would be much simpler leaving out the plugin builder. When introducing the loader, I would introduce the simple format first (without a plugin builder), and the pluginbuilder second as a (less elegant) convenience. But I do think that the convenience is the most important for the adoption.

I'm not sure how you would do this, it seems like providing more than one plugin. Maybe that will work out, but under a different repo? In general, I think it is good if there are not too many variants of the same thing in one repo, but if you see a way for it to work, it may be worth trying.

I think the main benefit of a generic 'transpiler' plugin is in widening the transpilers we have and ensuring they are of high quality, so there should be a focus on best practise here. This could potentially be used as a base for everything from require-css to handlebars to es6 shims.

Agreed, it is a great idea, and using the cs one as the start is a great way to test it out, particularly when looking ahead to things like sourcemap integration.

The Require-CS project is a great place to test it out. I will do some further testing and then would be keen to integrate. Let me know if you are happy in the current form or have any further feedback. Shall I push to master or a separate development branch?

If you have tested it out in your fork, I'm OK if you just push to master, do any readme updates, and do tags and a release. What I normally do is a git tag for the version release then a git tag -f latest, using the latest tag to always point the latest release.

I'm also open to moving this repo under another group, either requirejs/cs or even deprecating this fork in favor of yours. Looks like there is an admin option to transfer the ownership of this to another group/person, so moving it to requirejs/cs may be straightforward, but we could probably also work it out where if you remove your fork, I could then transfer it over to your name.

Also to make installation easier, how would you feel about adding a component.json as well as the volo properties?

I assume the component.json file would be for "bower" and not "component". If so, if you find it useful, go for it, as long as the package.json and component.json version/dependency sections are kept up to date.

guybedford commented

I've updated the Require-CS on my own branch here - https://github.com/guybedford/require-cs-amdcompiler. Will do further testing (including of the package management), and then I also need to review the readme.

Yes component.json would be for bower. It seems like this spec is quickly becoming the norm. In terms of the name, I think requirejs/cs would make the most sense as it is an official plugin.