MeteorCommunity / discussions

Track technical discussions in the Meteor community
89 stars 7 forks source link

Support ast build handlers #51

Open raix opened 9 years ago

raix commented 9 years ago

Why? - Let packages modify and improve code in steps,

Examples of packages that could do work on the code:

The quick fix just lets multiple source handlers look at the source - basicly to allow code reporting / jshint

Your use case could bring a really nice / important improvement - having eg. a source handler for replacing Meteor.isClient and Meteor.isServer with true/ false would be nice to run before eg. uglify (uglify would then remove the dead code/unused code blocks)

I'm not sure of a way to let the bundler sort source handlers - first thought was a unix like load levels init 1 - 5 each level runs source handlers unordered - but ordered to levels - but the levels would be triggy to define / standardize?

An other issue is that:

   Package.register_extension("js", jshintHandler);

jshintHandler is not handed the actual source?

var jshintHandler = function(bundle, sourcePath, serverPath, where) {

Could do something like:

   Package.register_extension("js", jshintHandler, initLevel);

where initLevel is some sortable level

  // Allowing packages to modify the AST
  var jshintHandler = function(bundle, sourcePath, serverPath, where) {
  bundle.source === theManHandledSourceSoFar;
  bundle.source = theNewUglifyedSource;

Something like that could be nice - but it would require some work - and extruding the uglify to a package - but this would be an improvement too?

Quotes taken out of context - read ref links to get it all:

@raix:

I wrote a Meteor-jshint package, To run jshint on .js files is currently not allowed since there can't be 2 or more source handlers. A source handler doesn't nessesarly change the code - Like in the jshint case - its just parsing and checking the code and reporting in on bad gramma. I kept the warning about "two packages ..." - it only fires once pr. extension to keep down the verbosity

@gadicc:

I'd like to add my support to this and provide another use case: The messageformat smart package currently extracts strings for translation from files using a CLI tool installed via npm. It would be super cool to rather do this via source handlers, making the process completely integrated and invisible to the developer.

@mquandalle:

Yes this is an idea that will really push the limit of the packages system further. I have a lot of ideas of packages that would benefit to be able to access or even modify the JavaScript code. This is a must-have for Meteor 1.0.

@ccorcos:

I totally agree! I think the biggest issue here is that source handlers go into an object. So the reason you can only have on source handler is the key will get overwritten. Seem like it should be an array as opposed to an object ;) In any case, I am looking for this feature as well so I can compile HTML, JS and CSS all from the same file! How amazing does that sound ;)

@glasser:

Actually this sounds like a reasonable enough idea. I am looking into implementing it with the new bundler. Agreed that package.js needs refactoring. The 0.6.5 "linker" release was originally going to involve a rewrite of package.js, but we've decided that in the interest of ever releasing it to push that to future work. But it does have a new (non-user-visible) package manifest file in the new built "unipackage" format.

Ref:

mquandalle commented 9 years ago

I've been trying to implement this functionality in the meteor tools, and actually my opinion has changed since our discussion last year in https://github.com/meteor/meteor/pull/1207.

I think it's a good model to have one source handler per extension. Basically we are using it to transform any language (html, coffee, coffee.md, jade, harmony, etc.) into [html]/css/javascript.

What would be useful I think is a way to take a complete compiled “image” (for instance the browser application) and modify it (for instance to remove Meteor.isServer code, or to do some minification, or any other transformation). The idea here is that we don't handle one particular file but all the code of a compiled application/target/image. This handler would work with the AST, so we would have to parse and re-compile the code only once (which IIRC is currently done by js-analyse package anyway, so we don't have more performance penalty by doing that).

Modifying the AST might not be the most simple task for package authors, but here come the excellent recast library by @benjamn (who is currently working at MDG).

The API could be something like Plugin.registerImageRecaster, to be consistent with the Plugin.registerSourceHandler. But actually I think the world register doesn't bring much in this context, so I would prefer to have something like:

In the recastImage method, the optional options parameter is used to specify an archMatching so we could target a particular image, and also to specify a priority (between 0 and 1) so we could sort recasters (as we already discussed in https://github.com/meteor/meteor/pull/1207 replacing Meteor.isServer by false on the server must be done before removing the dead code).

Open questions:

benjamn commented 9 years ago

One convenient property of Recast is that it doesn't care how you do the AST transformation, so I think it makes a lot of sense to give the plugin author the AST and let her decide how to modify it. If the author decides to use recast.visit, that's great (since I can provide better support for that), but any function that modifies the AST will work!

On the implementation side, we can certainly use recast.parse and recast.print before and after the transformation, but that's an implementation detail.

raix commented 9 years ago

@mquandalle Se see your point - Lets keep sourcehandlers be simple converters, one pr. file ext. I'll update the title and have it something like a build plugin:

This concept would be really powerful if html/css is also converted into js format?

raix commented 9 years ago

btw. @benjamn I read the recast code awhile back - I kinda liked the concept - It would be nice to use it in the build tool, would allow us to do crazy stuff

mquandalle commented 9 years ago

@benjamn, would you consider adding a package.js to recast? (#14). Then we could directly using the isopack loading method from the meteor core tools without the need to add one more npm dependency in the dev bundle.

mquandalle commented 9 years ago

@raix, I guess you would be able to rewrite https://github.com/raix/Meteor-famono require lookup using the AST instead of doing black magic on the client side; is that what you have in mind?

raix commented 9 years ago

@mquandalle Famono will deprecate if this became a feature - we could do alot of code optimizations eg. removing dead code and merge alike functions etc. This way we wont have to think about including the whole famous library.

But I've got a lot of other ideas for this feature too,

benjamn commented 9 years ago

@mquandalle definitely happy to add a package.js file (but also happy to take a PR if someone beats me to it)!

raix commented 9 years ago

It seems as if facebook is creating a system much like meteor - but with some very important improvements, many of those are sadly old feature requests to meteor that haven't been touched yet. They already support ast build handlers as a feature.

mitar commented 9 years ago

Which system are you talking about?

raix commented 9 years ago

The Facebook stack:

This FR is the type "Loaders"AST transformations in webpack

https://www.youtube.com/playlist?list=PLb0IAmt7-GS1cbw4qonlQztYV1TAW0sCr

mitar commented 9 years ago

But for server side they do not provide anything? Node? PHP?

steph643 commented 9 years ago

Just to connect the dots, here is the discussion about Meteor and React (started by @mitar): https://groups.google.com/forum/#!topic/meteor-talk/crD-tGGLDPY

raix commented 9 years ago

I'm not sure @mitar they mention it in the end of the Relay talk I think

Thanks @steph643 for the reference (I'm not subscribing to the meteor talk due to spam) additional ref: https://github.com/meteor/meteor/issues/3728

mitar commented 9 years ago

https://meteor.hackpad.com/Proposal-streams-based-build-tool-kbqhWoYYfKR

mquandalle commented 9 years ago

The stream based build tool proposal is an exciting architecture proposal! It seems that Babel transformers could be used to handle AST modifications, see https://speakerdeck.com/sebmck/babel-facebook-talk.

ccorcos commented 9 years ago

That would be awesome!