Skip to content
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

Made RequireJS friendly. #5

Closed
wants to merge 4 commits into from
Closed

Conversation

alexindigo
Copy link
Member

Wrapped into amdefine and "hid" require of server only file.

@@ -20,7 +25,8 @@ exports.templatePatterns = templateFinder.templatePatterns;
* `getLayout` should only be used on the server.
*/
if (typeof window === 'undefined') {
exports.getLayout = require('./server/layoutFinder')(Handlebars).getLayout;
var serverOnlyPath_layoutFinder = './server/layoutFinder';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So r.js wouldn't go down that route, it ignores require with variables.

@spikebrehm
Copy link
Member

I'm open to supporting RequireJS, but a little wary about having to add this boilerplate to every file in Rendr. Yikes!

Perhaps we can follow the example of some leading projects like Handlebars and actually just use ES6 modules, which can be transpiled to CommonJS & RequireJS (see this PR: handlebars-lang/handlebars.js#628).

Thoughts on that? It's initially more work, but ultimately more satisfying :)

@alexindigo
Copy link
Member Author

I absolutely agree, boilerplates suck.
Although in this case it's not as bad if you consider how it will be used.

  1. It's not all the files in the project but the ones in /client and /shared folders that are meant to be executed in async environment of the browser, which shows. And few core files, but I'm trying to eliminate it and load core as one solid piece.

So it does make sense from some angles of view.
Let me check that PR, I'll be back. :)

@alexindigo
Copy link
Member Author

It will require compilation step for both server and browser,
and if we just stick with CommonJS it will eliminate compilation step for the server,
but still you'd have to rerun grunt to see your changes on the client.
And AMD format allows you to enjoy your files (with real line numbers) directly.

For me it's not big of a problem to make client bound files make look more like browser based js,
especially in Rendr where it's the whole point.

Maybe if remove that scary comment wrappers and make normal indentation it would look more organic
and less disgusting.

I really want to escape compilation step for each change, I think it's the main fun killer,
and what's doomed YUI3.

@alexindigo
Copy link
Member Author

Hey, I've got idea for the middleground. :)

Since rendr doesn't really change for the app
and only parts of it should be visible from the web.

I'll have grunt script copying whatever needed to the public folder,
and wrapping stuff up on the way.

But we still need to keep changes server-only require statements.

What do you think?

@alexindigo alexindigo closed this Nov 6, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants