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

Provide example using Browserify #35

Closed
spikebrehm opened this issue May 6, 2013 · 11 comments
Closed

Provide example using Browserify #35

spikebrehm opened this issue May 6, 2013 · 11 comments

Comments

@spikebrehm
Copy link
Member

No description provided.

@jcblw
Copy link
Contributor

jcblw commented May 30, 2013

I would love this 😄 , whenever I try it seems to want to load express.

@spikebrehm
Copy link
Member Author

Yeah, I imagine we'd have to modify our approach to requiring modules. No small task to figure this one out!

@jlogsdon
Copy link
Contributor

I was able to get Browserify working with a little bit of effort. I've done it manually using API for now, but a Grunt task would probably be better.

https://gist.github.com/jlogsdon/5779710

It's not the prettiest, but it packages everything up and if I save the output to public/mergedAssets.js the app works fine. I had to make one change to rendr: globals.rendr.entryPath is no longer used when including app stuff as the modules are all available at app/name_of/module instead of a full path. This didn't appear to break anything server-side and browsing around works fine. I simply went through and replaced all entries of rendr.entryPath + "/ with " (one case was a single-quote).

edit: I've added a diff of the changes I made to rendr.

edit2: I just realized why this worked on the server-side: I forgot to restart the webserver. So the entry names will need some work still.

edit3: Okay, updated the script again so it works with rendr unmodified. App files will be available under /app with it, so an empty entryPath doesn't break things. Had to update __layout.hbs to call it correctly, though.

@amiorin
Copy link

amiorin commented Jun 17, 2013

I didn't try yet, but it's in my todo list. I'm interested too.
@jlogsdon
You coud use my grunt task for browserify : https://github.com/amiorin/grunt-watchify

@jlogsdon
Copy link
Contributor

Mine method is definitely a "Proof of Concept" as it doesn't sue Browserify "correctly" at all. Generally speaking you should be able to add a single entry point and let it go, but due to how a lot of the requires are handled I ended up creating "manual" packages for everything that can be required.

They got around this with the stitch implementation by creating a custom grunt task that did aliasing, but from what I can tell there's no simple way to do a similar aliasing method with Browserify.

@spikebrehm
Copy link
Member Author

Thanks @jlogsdon! This is a great start. I'd love to figure out how to get it working with using Browserify "correctly" -- with a single entry point. However, I expect this is still more flexible than Stitch. Mainly, I want to avoid having the app hardcode all of the NPM dependencies. At the very least, I would want Rendr to own that list, so the app developer can upgrade to a newer version of Rendr and they don't have to change to build script.

Before switching over to Browserify, I'd like to see an example of how to integrate with rendr-handlebars, as demonstrated in: #56

@hurrymaplelad
Copy link
Contributor

Not to slow momentum, but I'm not convinced adding Browsersify support is an improvement. Are we talking about supporting both Stitch and Browserify, or switching entirely to Browserify?

Since each tool adds it's own set of constraints for including and excluding modules, supporting both is significantly more complicated.

If we're talking about switching entirely to browserify, the dynamic requires done here and elsewhere in that same file would need extra build magic. Browserify doesn't understand them out of the box.

We'd also have to take extra care to keep server modules out of the build. A mistyped require could explode the bundle size without warning. Projects using the framework would inherit the same challenge.

Other potential solutions to polluting project build files:

Maybe there's some combination of module-deps and browser-pack that will do just enough.

Maybe we could expand grunt-rend-stich to parse a per-package manifest of files to be sent to the client. rendr-stitch-manifest.json or somesuch.

Imagine a <my-project>/Gruntfile.js like

  rendr_stitch: {
      compile: {
        options: {
          dependencies: [
            'assets/vendor/**/*.js'
          ],
          npmDependencies: {
            rendr: true,  // flag to grunt-rendr-stitch to look for a rendr-stitch-manifest
            rendr-handlebars: true
          }
        },
        files: [{
          dest: 'public/mergedAssets.js',
          src: [
            'app/**/*.js'
          ]
        }]
      }

and a rendr/rendr-stitch-manifest.json

{
  npmDependencies: [
    underscore: 'underscore.js',
    backbone: 'backbone.js',
    handlebars: '../rendr-handlebars/node_modules/handlebars/dist/handlebars.runtime.js',
    async: 'lib/async.js'
  ],
  aliases: [
    {from: './client', to: 'rendr/client'},
    {from: './shared', to: 'rendr/shared'}
  ],
  files: [
    'client/**/*.',
    'shared/**/*.js'
  ]
}

I don't have a strong opinion on the right direction, just want to make sure we consider options before committing to a direction. Browserify, by default, assumes the modules it's bundling are going to the client in their entirety. Rendr modules, part client, part server, part shared seem like new territory.

@spikebrehm
Copy link
Member Author

Just to follow up on this, I think you're on the right path @hurrymaplelad. I do like the idea that each module can expose a manifest listing its packaging requirements. If it weren't very verbose, it could live directly in package.json, but perhaps it's better to live separately. I'd also be neat to find a format that is independent of Rendr, and more general.

@akbarahmed
Copy link
Contributor

I have a solution working for Browserify with the example 00 using the code pulled from npm install. There is a bug when I apply the update to the Rendr version in master, but I plan to push this into my repo later today. It's a clean implementation with only minor modifications to the Rendr source.

@akbarahmed
Copy link
Contributor

I'm ready for people to review Rendr with Browserify. I updated both rendr and rendr-handlebars. Currently, I only implemented the update for example 00_simple.

This is not yet ready for a pull request as the npm package / tag / version for both rendr and rendr-handlebars will need to be updated at the same time this is merged into master. The reason is that rendr and rendr-handlebars are both dependencies in 00_simple package.js. Further, rendr-handlebars is a dependency listed in rendr's package.js.

Steps to Test

git clone https://github.com/akbarahmed/rendr.git

cd rendr/examples/00_simple && npm install

cd node_modules && rm -r rendr rendr-handlebars

git clone https://github.com/akbarahmed/rendr.git

cd rendr && npm install && cd ..

git clone https://github.com/akbarahmed/rendr-handlebars.git

cd rendr-handlebars && npm install && cd ..

cd rendr/node_modules && rm -r rendr-handlebars

git clone https://github.com/akbarahmed/rendr-handlebars.git

cd rendr-handlebars && npm install && cd ..

cd ../../..

grunt server

Random note: If anyone knows how to debug Browserify I'm all ears. Getting this working took 3 days, and in the end it was trival...although debugging was a PITA.

@lo1tuma
Copy link
Member

lo1tuma commented Feb 6, 2014

Fixed by #192.

@lo1tuma lo1tuma closed this as completed Feb 6, 2014
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

No branches or pull requests

7 participants