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

Move BrowserSync back into core #609

Closed
bmuenzenmeyer opened this issue Feb 6, 2017 · 8 comments
Closed

Move BrowserSync back into core #609

bmuenzenmeyer opened this issue Feb 6, 2017 · 8 comments
Assignees
Labels
pinned 📌 Don't let stalebot clean this up staged for next release 🏁
Milestone

Comments

@bmuenzenmeyer
Copy link
Member

Part of #603

@bmuenzenmeyer bmuenzenmeyer added this to the 3.0.0 milestone Feb 6, 2017
@raphaelokon
Copy link
Contributor

I can port the stuff I have in the CLI for serve or the stuff we have in the edition for serving into the core. I guess patternlab node-team need to decide on the serve API here and its responsibilities.

@geoffp
Copy link
Contributor

geoffp commented Feb 6, 2017

I suggest that to build arbitrary toolchains from inside any JS tooling, be it Gulp, Grunt, Broccoli, my own app, or whatever, I need to (at least) be able to:

  1. Configure Pattern Lab's BrowserSync however I want. I suggest that our current config becomes a defaults object, and the host JS can override this as needed by providing any subset of it.
  2. Trigger just BrowserSync's reload() method whenever an event occurs in my toolchain. For instance, if I'm using SCSS + autoprefixer, postcss, or some such thing, I need to set up my own file watches for .scss or .css, and tell BS to refresh my browser views once my CSS pipeline has finished its build.
  3. Trigger a pattern rebuild when an event occurs in my toolchain. This would be used for situations in which patterns need to change in response to conditions in the CSS toolchain, such as style inliners for HTML email authoring, or for some of the CSS-in-JS technologies from the React world.
  4. Catch events that come from PL in my toolchain. If I want to integrate a JS linter (for React components or other case in which my patterns are JS-based, an HTML linter (for plain template-y patterns), or any other testing tool, my toolchain needs to know when a pattern file has changed so I can run those tests against it. This could be done by PL returning a Node stream that users could use to raise events, but that may not allow the toolchain to halt the build if there's an error. Maybe that's okay. We could also do this without a stream by providing callbacks to PL at init time.

@raphaelokon
Copy link
Contributor

@geoffp Your comment is gold!

@raphaelokon
Copy link
Contributor

  1. Sounds fairly easy and I consider this a low-hanging fruit. And yes, a sensible default would be great.
  2. & 3. Seems to be stuff related to a specific toolchain. So the toolchain calling PL's API like build or reload
  3. Is PL currently built on an event-based architecture?

@geoffp
Copy link
Contributor

geoffp commented Feb 6, 2017

Is PL currently built on an event-based architecture?

It is not, but in this hypothetical, I think these events would come from the new file watchers, which are, and would act as the execution root in this case, emitting events to the returned stream and calling PL methods in response to filesystem events.

@raphaelokon
Copy link
Contributor

This is great and is no show stopper for the CLI – one could hook up additional commands to run on certain events from a command-line interface or pipe file paths to do further operations.
As long as the event-based architecture is not build around a certain toolchain/framework it also should be pretty flexible.

[...] my toolchain needs to know when a pattern file has changed so I can run those tests against it
Sounds a bit like a toolchain could figure that out by itself or streaming those paths outworn we have watches integrated into core

@stale
Copy link

stale bot commented Oct 2, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the needs response label Oct 2, 2017
@bmuenzenmeyer bmuenzenmeyer added pinned 📌 Don't let stalebot clean this up and removed needs response labels Oct 2, 2017
@bmuenzenmeyer
Copy link
Member Author

Brian to address #609 (comment) soon.... 🚀

bmuenzenmeyer added a commit that referenced this issue Oct 12, 2017
bmuenzenmeyer added a commit that referenced this issue Oct 12, 2017
part of #609
part of #706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned 📌 Don't let stalebot clean this up staged for next release 🏁
Projects
None yet
Development

No branches or pull requests

3 participants