-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preprocessor modules #70
Conversation
@pushred review pls |
@@ -284,14 +289,18 @@ Processed context in `/kitties` | |||
} | |||
``` | |||
|
|||
Preprocessors also come preloaded with some popular js libraries by default. [Underscore](http://underscorejs.org/), [Moment](http://momentjs.com/), [XDate](http://arshaw.com/xdate/), and [Sugar](http://sugarjs.com/) are all automatically accessible within preprocessor files. Here's a quick example: | |||
By default, the following libraries are available for use in preprocessors by using the `require` method: [Underscore](http://underscorejs.org/), [Moment](http://momentjs.com/), [XDate](http://arshaw.com/xdate/), and [Sugar](http://sugarjs.com/). Additional node.js libraries can be added by adding them to your site's `package.json`, installing them, and `require`ing them in your preprocessor. Here's a quick example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This contradicts what you said in the PR while the example below reinforces it:
Libraries like Sugar, Underscore, etc are no longer included by default
What's it gonna be? Personally I don't think we should include any library by default as ES5 has got quite a bit to offer on it's own and we'd be presumptuous about what the preprocessor would be used for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are not "included" by default, but they are "available". Basically this just means that you have to require
them, but you don't have to add them to the package.json
yourself. Other libraries will have to be added manually.
One thing I'm seeing in this branch is I no longer get this in the console:
I only get:
|
Also, please rebase. The caching in dev is making testing a pain. |
@pushred do you like now? |
@pushred I am going to go ahead and merge this into master this week. I can't just let this sit around. |
update tests
This is now running live on http://keithurban.net and doing just fine. While it's not quite as performant as I had hoped, it's still an improvement over our past implementation. |
This should keep preprocessors from catching on fire, but requires a significant amount of work from developers to understand what's going on. The changes include:
My main concern here is that people writing preprocessors will need to know how to write node modules... which isn't necessarily a bad thing, but another small hurdle to jump over on the way to making a site. Ideally, users will never have to make preprocessors (especially once we have better helpers in place), but we will certainly need to add better instruction, nonetheless.
This is also a massively breaking change, so we'll need to bump the version up pretty aggressively.