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

Integrate with browser-sync #367

Closed
jescalan opened this issue Jan 9, 2014 · 24 comments
Closed

Integrate with browser-sync #367

jescalan opened this issue Jan 9, 2014 · 24 comments
Labels

Comments

@jescalan
Copy link
Owner

jescalan commented Jan 9, 2014

This would theoretically allow us to strip all the live reload stuff out.

BrowserSync/browser-sync#30

@zspecza
Copy link
Contributor

zspecza commented Jan 9, 2014

And add multi-device testing and CSS injection on top of just a simple live reload

@zspecza
Copy link
Contributor

zspecza commented Jan 14, 2014

Just worth noting: browser-sync doesn't necessarily have to replace the watcher. It can, but it can also forward to an existing server process through a proxy. This might make running tasks easier, and will still allow for custom dev paths

@jescalan jescalan added the v3 label Feb 27, 2014
@dkmooers
Copy link

Big +1 for this - I really enjoy being able to use this with Metalsmith via Gulp. I can have Safari, Firefox, and Chrome open on my Mac and then an Android phone and iPad all running simultaneously - big productivity boost for responsive testing. I'd be willing to take a stab at this if someone could point me in the right direction?

@jescalan
Copy link
Owner Author

I think this would be the spot to start 😀

https://github.com/jenius/roots/blob/v3/lib/local_server.coffee

@jescalan
Copy link
Owner Author

Ping for progress here, this would be still great to have!

@jescalan jescalan changed the title v3: integrate with browser-sync Integrate with browser-sync Dec 17, 2014
@jescalan jescalan removed the v3 label Dec 17, 2014
@dkmooers
Copy link

Sorry, I'm out on this one, I'm not using Roots anymore - I'm sticking with WP for now, and using Meteor for an interactive app. Best of luck!

@jescalan
Copy link
Owner Author

Cool, thanks for the heads up @devinmooers. We have an idea for integrating wordpress but compiling out as static through roots in the pipeline too so don't tune out entirely 😀

@dkmooers
Copy link

Sounds great, thanks for the heads-up!

@zspecza
Copy link
Contributor

zspecza commented Dec 18, 2014

I'm not sure if this would fit better as an extension or part of Roots core...

@jescalan
Copy link
Owner Author

It's got to be roots core. It would replace the current watch and reload system.

@dbox
Copy link

dbox commented Aug 7, 2015

@hhsnopek Is this something in your wheelhouse? Was just using another project with browsersync and it's soo fast.

@jescalan
Copy link
Owner Author

Still would like to make this happen. Eventually will look into it when I have time, promise!

@zspecza
Copy link
Contributor

zspecza commented Aug 18, 2015

This will be on my radar when I start :)

@jepedo
Copy link

jepedo commented Sep 17, 2015

Integrating browsersync would be slick. +1 for this.

@jescalan
Copy link
Owner Author

So browsersync has it's own server, which would mean dropping charge and causing a number of breaking changes. However, its server accepts middleware, so it appears as if we could make it behave exactly like charge by just importing charge's middleware.

I think this won't be super difficult. Going to take a stab at it soon!

@jescalan
Copy link
Owner Author

So integration is moving along. The issue though is the same issue we have with our home-made reloading system, the dependency tree and compile pipeline.

Let me explain. When you reload browser-sync, you can pass it the name of a file or multiple files to reload. So for example if you detect that foo.css changed, you would pass browsersync something like bs.reload('foo.css') and it would do its thing with the injection etc. Now here's the issue. Let's say you make changes to a jade layout file. We would have detected a change to layout.jade. However, this needs to be changed in a number of ways. First, it needs to be translated to its output path, which in this case is nothing since the file is ignored. Then it needs to pull in all its dependencies (every other file that uses the layout), which we have no way of attaining, and it needs to reload those files.

So basically, we cannot use browsersync to its fullest speed until all roots-supported compilers also output dependency trees, or we devise a method to make them all do this behind the scenes, and then after that until we make another set of changes to roots that allows us to track and use these trees for reloads and compiles.

That being said, there are two things that can be done here. The first is to try to approximate a couple use cases in which we can in fact reload just a single file, and for any cases where we can't be sure, reload everything. This was what we did in the previous version of roots, and while it ensured quicker reloads for a small number of use cases (like a single html view being changed), it also produced a number of bugs for edge cases where dependency reloads were not triggered and the watcher would have to be restarted. In this version, we decided to reduce bugs and simply reload everything each time the site changes.

For now, I'm going to move ahead with the integration and simply set it to reload everything each time, as it already does. As improvements come down the line and eventually we can perhaps make use of dependency trees, we will be able to take advantage of a little more speed out of browsersync, which will be great. Until then, the functionality will be there waiting for us.

Further discussion of the "dependency tree" issue can be found here for those interested and following along! #586

@calebeby
Copy link

How can BrowserSync be configured in Roots?

@calebeby
Copy link

I want the browsersync script in head instead of body:

snippetOptions: {
  rule: {
    match: /<\/head>/i,
      fn: function (snippet, match) {
        return snippet + match;
      }
  }
}

from BrowserSync/browser-sync#977 (comment)

@jescalan
Copy link
Owner Author

@calebeby currently there isn't an option to configure browsersync, although if you'd like to make a pull request to add one you are welcome to, the logic is here. I'm not sure I understand the need to do this though, isn't this an issue with rails/turbolinks?

@calebeby
Copy link

calebeby commented Sep 26, 2016

Sure, I'll see if I can work on it this weekend. Turbolinks can be used outside of rails, it is just a script that intercepts links like spike-pushstate.

It does not play nice with browsersync because browsersync puts the script
in body instead of head

@jescalan
Copy link
Owner Author

@calebeby honestly it would probably be better to just use spike-pushstate. I can fix it to work with the latest version if you want.

@calebeby
Copy link

calebeby commented Sep 27, 2016

Can spike-pushstate work with roots?

@calebeby
Copy link

Even if spike-pushstate does work with roots, it is still a good idea to make browsersync configurable. Are you still interested in me working on this?

@jescalan
Copy link
Owner Author

jescalan commented Sep 27, 2016

@calebeby it will definitely not work with roots (sorry, i was confused haha), but this project would!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants