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

SSR & file-based dynamic routes in src/_routes #383

Merged
merged 126 commits into from
Oct 6, 2021

Conversation

jaredcwhite
Copy link
Member

@jaredcwhite jaredcwhite commented Sep 6, 2021

Builds upon (and supersedes) PR #281 and resolves #379.

Lots to go on this but early results are very promising…

Here's what the server/roda_app.rb file looks like:

require "bridgetown-routes"

class RodaApp < Bridgetown::Rack::Roda
  plugin :bridgetown_ssr
  plugin :bridgetown_routes

  route do
    # Process routes in server/routes and src/_routes
    Bridgetown::Rack::Routes.start! self
  end
end

Here's what an example route looks like (src/_routes/items/[slug].erb):

---<%
r.get do
  params = r.params[:slug].split("-")
  render_with data: {
    layout: :page,
    title: "The ID…",
    results: "The ID is: #{params[0]}. Slug is: #{params[1]}"
  }
end
%>---

<p>The results are in:</p>

<p><strong><%= resource.data.results %></strong></p>

<pre>
  <%= resource.id %>
  <%= resource.relative_path %>
</pre>

jaredcwhite and others added 30 commits April 12, 2021 10:20
* Relations for resources (belongs_to, has_many, etc)

* improve resource type comments

* Make relations available to Liquid templates

* remove stray p methods

* Add test for resource relationships

* Support pluralized belongs_to keys

* make cop happy

* Use model origin id for resource id

* Add documentation and Liquid test for resource relations

* use newer feed gem

* Use list for relation accessor examples

* Fix nil bug in relations schema
* New Component class with a ViewComponent-ish API

* Isolate site in components and improve link helpers

* Remove unnecessary special render case for ViewComponent

* Fix missing variables

* Relocate markdownify to Helpers class

* Switch to new Rails-like OutputBuffer, more VC support

* Support clean render interop between builtin Component and VCs

* Add several common Liquid filters in as Ruby helpers

Also add output_safety dependency

* Additional html_safe usage

* Match Rails' capture escaping logic

* Change before_render behavior to match latest VC

* Add tests and code comments for Bridgetown::Component

* Couple of YARD updates

* fix cop offense

* Fix bug with url_for helper

* Correct failing ERB feature test

* Use new SEO and feed helpers for the website

* Fix odd cop errors

* Convert website navbar to ERB component

* fix is-active class name

* Use resource variable in default website layout

* Update documentation (WIP) on components subsystem

Also change all occurences of Javascript to JavaScript

* Add lint-html addition

* Add documentation regarding ViewComponent

* Make linthtml happy

* Fix typos

* Add link to components docs from the ERB page

* Add section on ERB output safety to the docs
* Major feature addition for Ruby Front Matter and raw templates

Also additional work to normalize Liquid & ERB template APIs

* Improve docs

* improve error messages, support Ruby data files

* Add to_json support for Resources

* Improve code quality and test rbfm

* Refactor front matter importing and add rbfm to layouts

* Revert and use regex capture

* Remove logging around rbfm

* Lots of Ruby files and rbfm documentation, couple of fixes

* Better to_json compat

* Fix Liquid error on website build
* Fix bug which was swallowing dotfiles and multiple extensions

* Revert tag refactoring
@jaredcwhite jaredcwhite changed the title WIP: SSR & file-based dynamic routes in src/_routes SSR & file-based dynamic routes in src/_routes Sep 24, 2021
@jaredcwhite
Copy link
Member Author

Note to self: fix failing tests before final merge (duh)

Copy link
Member

@ayushn21 ayushn21 left a comment

Choose a reason for hiding this comment

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

Awesome stuff. That was a mammoth of a PR :D.

I can't say I understood everything but there was nothing bad that jumped out at me. Looking forward to having this in main!

Do you think it's worth duplicating bin/bridgetown to bin/bt? It's a tiny amount of code that's unlikely to change so duplicating the file isn't the big deal in my opinion. It'll avoid putting the onus on the developer to set up an alias and provide a concise way to invoke commands out of the box.

@@ -4,7 +4,6 @@ inherit_from: ../.rubocop.yml
AllCops:
Include:
- lib/**/*.rb
- spec/**/*.rb
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth replacing with test/**/*.rb instead of deleting this line?


# Puma is a Rack-compatible server
# (you can optionally limit this to the "development" group)
gem "puma", "~> 5.2"
Copy link
Member

@ayushn21 ayushn21 Oct 5, 2021

Choose a reason for hiding this comment

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

Maybe it's worth putting this in development by default? I'd imagine the vast majority of people would only use it locally.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the question is which DX is worse: installing the puma gem in production even if it's not required for a static site build, or not installing it even when someone might need to utilize SSR? I tend to think the latter would be annoying and we'd have to document it: "hey, wanna do SSR? you need to move puma out of development in the Gemfile…" hmm

Copy link
Member

Choose a reason for hiding this comment

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

Yeah fair point. It's not doing any harm outside the dev group and saves the need to go and look up some docs 👍

@ayushn21
Copy link
Member

ayushn21 commented Oct 5, 2021

Just thinking out loud, I have no idea if this is even feasible.

For live reloading, I wonder if it'd be possible to use EventSource to write a message to the client to refresh itself? We'd need to create the connection to a live_reload endpoint and listen for messages; that bit is fairly trivial. We'd also need to have some kind of file watcher and write a message to the client when a file changes. Not sure how feasible that is.

Seems like it'd be an easier way to go about it than using websockets; and also a bit more technically "correct" I suppose since the client never needs to write anything in a live reloading scenario.

The polling approach in this PR at the moment works so definitely think we should proceed with that; but perhaps something to think about for the future?

@jaredcwhite
Copy link
Member Author

For live reloading, I wonder if it'd be possible to use EventSource to write a message to the client to refresh itself? We'd need to create the connection to a live_reload endpoint and listen for messages; that bit is fairly trivial. We'd also need to have some kind of file watcher and write a message to the client when a file changes. Not sure how feasible that is.

TIL about EventSource!

I'm totally open to any push-based mechanism in addition to polling, or instead of (if it's reliable enough over non-localhost connections).

@jaredcwhite
Copy link
Member Author

Do you think it's worth duplicating bin/bridgetown to bin/bt? It's a tiny amount of code that's unlikely to change so duplicating the file isn't the big deal in my opinion. It'll avoid putting the onus on the developer to set up an alias and provide a concise way to invoke commands out of the box.

I like that idea!

@ayushn21
Copy link
Member

ayushn21 commented Oct 6, 2021

TIL about EventSource!

I'm totally open to any push-based mechanism in addition to polling, or instead of (if it's reliable enough over non-localhost connections).

I discovered them just yesterday myself! I stumbled upon this PR which adds an EventSource source to Turbo: hotwired/turbo#415

I'll create an issue for this once this PR is in main .... and add it to my never ending list of things to do :D

@KonnorRogers
Copy link
Member

TIL about EventSource!
I'm totally open to any push-based mechanism in addition to polling, or instead of (if it's reliable enough over non-localhost connections).

I discovered them just yesterday myself! I stumbled upon this PR which adds an EventSource source to Turbo: hotwired/turbo#415

I'll create an issue for this once this PR is in main .... and add it to my never ending list of things to do :D

You may know EventSource under a different name, it's also called "Server Sent Events" we used them at Veue for our chat system.

https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events

@jaredcwhite jaredcwhite merged commit 2988a48 into main Oct 6, 2021
@jaredcwhite jaredcwhite deleted the bridgetown-file-routes branch October 6, 2021 18:49
@ricsdeol
Copy link

Hi, this is amazing project, but I didn't find how I render this _route/item/[slug].erb ?

I tried to use this example:

server/routes/count.rb

# Rename this file to hello.rb to try it out

class Routes::Count < Bridgetown::Rack::Routes
  route do |r|
    # route: GET /count
    r.get "hello"  do
      { oi: "friend from server" }
    end
  end
end

src/_routes/count/hello.liquid

<p><strong>Oi:</strong> {{ resource.data.oi }}</p>

src/_layouts/default.liquid

 {% render "count/hello" %}
 {% render "routes/count/hello" %}

And I get this error:

[Bridgetown]   Liquid Exception: Liquid error (line 10): No such template 'count/hello' in /home/ricsdeol/projects/leaning/count_teste/src/_layouts/default.liquid

Could you please give some examples using data from server ?
Is the server intended to be accessed only through JavaScript request?

@jaredcwhite
Copy link
Member Author

@ricsdeol In case you didn't see my reply on Discord…

I think you don't need any of the examples except just the single src/_routes/count/hello.liquid file, and it should look something like this: https://gist.github.com/jaredcwhite/b14285934746202236db0b9bbdf2071a

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.

feat: File-based SSR routes
4 participants