withastro / roadmap

Ideas, suggestions, and formal RFC proposals for the Astro project.
290 stars 29 forks source link

Partials RFC #721

Closed matthewp closed 10 months ago

matthewp commented 11 months ago

Summary

Add the ability to configure a page in the pages directory to be treated as a fragment. A fragment is a chunk of HTML that is not interpreted to be a page, so no head content or doctype are included.

Links

ZebraFlesh commented 11 months ago

No client-side scripts from Astro will be part of this change.

Can you elaborate on the meaning of this line from the Non-Goals section? My use case is that I want to include client side scripts defined by my fragments, and I'm unclear on if this will/won't be supported.

philipschm1tt commented 11 months ago

I am interested in fragments coming from the micro-frontend perspective: include a fragment either server-side/edge-side via SSI/ESI or include a fragment client-side via Ajax. I started a roadmap discussion about these use cases at https://github.com/withastro/roadmap/discussions/713 Note: I don't expect these use cases to be covered with this planned partials feature but I think it's desirable to support these use cases in the future and thinking about that future can help inform the current first step towards partials.

The proposal solves some use cases: notably the HTMX/Hotwire-style approach where the server updates the markup for parts of the page.

I want to think one step further about potential future micro-frontend Ajax support. With Ajax, the "current" page might be rendered by any technologies geared for server-side rendering. Maybe a classic Java + Spring MVC page that uses a Thymeleaf template. Then you might have a button on that page to, for example, get your notification or look into your cart. That could be done via Ajax: the client-side JavaScript could request that fragment from a separate Astro server. The Astro server would then need to deliver not just the markup but also the styles and some JavaScript for that fragment. I believe those stylesheets and scripts are commonly then not added to the head of the page but should work by including the tags "inline" where the fragment is added.

If Astro were to support – in the future – that use case of delivering a fragment to a page rendered by another server, it would need to support styles and scripts for the fragment. Would you then add another flag like export const fragmentWithScriptsAndStyles that is false by default? Or would you include a second flag like export const fragmentMarkupOnly that is true by default? Or would you use two independent flags like export const fragmentMarkupOnly and export const fragmentWithStyleAndScript? If you would use two independent flags, the current proposal of just export const fragment could become ambiguous and need some work.

matthewp commented 10 months ago

@ZebraFlesh No, all I mean by that is that Astro is not opinionated about how you load a fragment in the client and are not getting involved in that. So Astro is not going to be adding scripts of its own. You can of course use your own scripts to manage fragment insertion.

matthewp commented 10 months ago

@philipschm1tt Scripts and styles are included without any config value, by default, for all pages. So if you want those things then you don't need this feature. Am I misunderstanding something here?

philipschm1tt commented 10 months ago

@philipschm1tt Scripts and styles are included without any config value, by default, for all pages. So if you want those things then you don't need this feature. Am I misunderstanding something here?

If I understand the proposal correctly, with a fragment, you would not get any stylesheet or script links as part of the fragment, but you would rely on the page that includes the fragment (where Astro presumably already provided styles and scripts for that page).

What I'm saying is that there is a fragment use case, where the page that includes the fragment does not come from Astro but from a different server. For that use case, it is common that the fragment that you include also contains a stylesheet and script.

For that micro-frontend Ajax approach, I imagine the ideal behavior like this: The fragment would be rendered like a page but instead of doctype, head, body, ... you would get a partial of the page body. That partial would contain a stylesheet link and script link as part of the fragment that would not be merged into the head of the page.

Here is an excerpt from the book Micro-frontends in Action (with a manning account, you can "unscramble" it for a few minutes): https://livebook.manning.com/book/micro-frontends-in-action/chapter-3/

Their fragment (in this example without JS) looks like this – it includes a stylesheet link in the fragment:

<link href="http://localhost:3002/static/fragment.css" rel="stylesheet" />
<h2>Recommendations</h2>
<div class="recommendations">
  ...
</div>

Just to reiterate: I'm not arguing for perfectly solving micro-frontend scenarios as part of this RFC but to consider in this RFC what the impact might be if you addressed the micro-frontend use cases in the future.

matthewp commented 10 months ago

@philipschm1tt I understand, what i'm saying is that the thing you are describing is not a fragment (in the definition of this RFC). It's just a regular page. You might think of it as a fragment, from the perspective of how you're using it within another framework, but Astro's perspective a page which includes styles is not a fragment.

Does that make sense? What I'm saying is, you should be able to create a page with just the content in your example and it will include styles, as you have shown. Without needing this feature.

philipschm1tt commented 10 months ago

@philipschm1tt I understand, what i'm saying is that the thing you are describing is not a fragment (in the definition of this RFC). It's just a regular page. You might think of it as a fragment, from the perspective of how you're using it within another framework, but Astro's perspective a page which includes styles is not a fragment.

Does that make sense? What I'm saying is, you should be able to create a page with just the content in your example and it will include styles, as you have shown. Without needing this feature.

Yes, I understand what use case(s) you want to solve with this proposal. But multiple people in the original discussion https://github.com/withastro/roadmap/discussions/266 had the micro-frontend use case. What bothered them (and me), was the parts of the page that only work at the page level and not as part of a fragment.

These server-side integration use cases (like the Ajax client-side integration approach), still need styles and behavior as part of these kinds of fragment. The fragments in these contexts come from different servers.

matthewp commented 10 months ago

@philipschm1tt Can you explain why the existing pages folder behavior does not work for your use-case?

philipschm1tt commented 10 months ago

@philipschm1tt Can you explain why the existing pages folder behavior does not work for your use-case?

I have not personally explored this as much as the commenters in https://github.com/withastro/roadmap/discussions/266 and some of my colleagues.

What I'm looking for is a response like

<link href="fragment.css" rel="stylesheet" />
<script defer src="fragment.js"></script>
<h2>Recommendations</h2>
<div class="recommendations">
  ...
</div>

So roughly the same behavior as for a page but without doctype and with the CSS/JS links for that "page" outside of a head tag.

And unlike some of the commenters on that issue, I would like to use SSR and could not use some of the post-processing workarounds that were suggested for static files.

matthewp commented 10 months ago

@philipschm1tt If you don't include a <head> tag then there will be no head tag, but there will be the scripts and styles at the top, just as you have written.

The only part that is different is that there will also be a doctype, which you say that you do not want.

I would then ask, how are you dealing with these scripts and styles? Are you not extracting them out and putting them into the head? If you do not, you will get FOUC.

ZebraFlesh commented 10 months ago

I am inlining those styles for the microfrontend use case. Today, I do the following:

  1. manually inline CSS
  2. remove DOCTYPE
  3. remove head tag (but retain contents)
  4. remove body tag (but retain contents)

And now I have a fragment. (I could skip the inlining, but since I'm doing ESI processing at edge on a streaming response, I can't "go back" and put the CSS in the head tag, because that content has already been streamed to the client.)

My hope for this feature was removing the last 3 steps because Astro would be smart enough to not add them.

matthewp commented 10 months ago

Astro does not add a head or body tag. So those steps you already can skip by simply not adding those tags yourself.

By (1) do you mean you take the link tags from the HTML and move them into the head?

matthewp commented 10 months ago

This feature can be tried out by installing the preview release:

npm install astro@next--fragments
philipschm1tt commented 10 months ago

The only part that is different is that there will also be a doctype, which you say that you do not want.

Yes. I think that's why that was the focus of https://github.com/withastro/roadmap/discussions/266 I think the SSI/ESI user might be helped best with a simple "please don't include doctype" config.

I would then ask, how are you dealing with these scripts and styles? Are you not extracting them out and putting them into the head? If you do not, you will get FOUC.

For the Ajax case the fragment is requested client-side on user interaction, so you won't have the styles and scripts during page load.

For the server-side integration case, this may depend on the specific approach you use. I'm not aware of a "built-in" way with SSI or ESI to move such links to the head. I don't know if some people might include a "fragment" that includes only these links to the head (at the cost of an additional request). I guess with SSI/ESI you may typically either accept some FOUC or use some workarounds to hide content until CSS is loaded (and then trigger the reveal with JS).

I think this is probably a classic SSI/ESI challenge with some options and trade-offs. It may not be a big deal since the main part of the page should usually come from the server that adds its styles/scripts to the head. And it may benefit from caching the CSS from the second request on.

ZebraFlesh commented 10 months ago

Astro does not add a head or body tag. So those steps you already can skip by simply not adding those tags yourself.

Excellent point. It's been a while since I've looked at this problem, so I was quoting from out post-processing script, but this explanation makes sense.

By (1) do you mean you take the link tags from the HTML and move them into the head?

No, I mean we find each <link rel="stylesheet">, read the indicated file contents, and replace the <link rel="stylesheet"> with <style>{contents_of_file}</style>. This is all done as part of an astro:build:done hook.

MrHBS commented 10 months ago

Consider renaming this feature to partials

matthewp commented 10 months ago

@MrHBS definitely open to doing that. Should probably run a pole some where.

matthewp commented 10 months ago

I've renamed it to Partials in this RFC. I'm working to also update the code. Additionally I've added a section to the RFC describing why the proposal doesn't include scripts/styles: https://github.com/withastro/roadmap/blob/fragments/proposals/partials.md#including-head-content-but-not-doctype

matthewp commented 10 months ago

@ematipico Added that in 47ab4ba772319a6453429fae40fcb7670861896e, let me know if that's what you were looking for!

matthewp commented 10 months ago

I'm moving for a call for consensus on this RFC. This will be the final comment period (3 days); if there are no objections this will be merged and the feature can be released in a future release (likely 3.4)

silent1mezzo commented 10 months ago

I think my biggest concern is with the variable vs file convention. Having it as a variable could clash with existing pages, third party components/packages and it also makes it more difficult to know whether a certain page is used as a partial or not since you have to investigate all of the pages individually.

Hugo handles partials by requiring them to be within a specific directory which is a little more elegant (albeit more work as mentioned in the RFC).

Any file nested under pages/partials would be treated as a partial and could be removed from routing. IMO this makes it easier to see which files are partials and can't conflict with any variables used in the component script.

ZebraFlesh commented 10 months ago

Just to provide some flavor, my plan is to use this implementation to build only partials (no actual pages). Think of it as: a build for a set of ESI replacements. Provided that's still possible with a directory-based approach, that looks good from the consumer side.

matthewp commented 10 months ago

@silent1mezzo I will probably use a src/pages/partials convention in my own apps, but I don't think Astro should enforce that. For a few reasons:

silent1mezzo commented 10 months ago
  • Astro is mostly unopinionated about where you put your files. Instead we lean on conventions; for ex. it's common for layouts to be in src/layouts, but they don't have to be! Layouts are really just normal components. So it's not normal for us to have a folder convention for organizational reasons. If there was a technical reason for it, we would do it.

That's fair

  • Adding a src/pages/partials known folder would be a breaking change, as people might currently have pages in such a folder.

Won't adding the partials variable also be breaking since someone could have that already?

  • If you didn't know of this feature, and added a page to src/pages/partials it would be confusing to see that the page did not include styles+scripts. Since this is a niche feature, I don't think having people use it by accident is a good thing to do.

Hmm, I actually disagree with this. I think the variable is more confusing than a folder. At best they're the same and you need to go to the docs to figure it out. At least the folder has examples from other frameworks people could lean on.

  • With the current design, integrations can add partials; they would not be able to do so with a folder or file naming requirement.

Totally fair

matthewp commented 10 months ago

Won't adding the partials variable also be breaking since someone could have that already?

No, exporting variables is not supported by Astro component outside of the documented ones (getStaticPaths, prerender), so we can add new variables as needed.

Hmm, I actually disagree with this. I think the variable is more confusing than a folder. At best they're the same and you need to go to the docs to figure it out. At least the folder has examples from other frameworks people could lean on.

A better explanation for my point here is that in Astro you can create any URL structure you want, and if we give special meaning to pages/partials that means that you cannot create a page with the URL /partials/whatever.

Conversely we would be enforcing a URL structure for partials themselves. Some people might want to do RESTful URLs where /todos is a partial response with the list of todos. I don't think it's Astro's place to tell people what their URLs can be.

silent1mezzo commented 10 months ago

Makes sense

lloydjatkinson commented 10 months ago

Just wanted to comment I find the name of the feature unclear (though I understand how the name was arrived at). Partials is already a fairly well used terminology to describe reusable templates or "views" particularly in MVC-like frameworks. These partials have little in common with this RFC. Essentially partials in this context are components. As a new user, I would be initially confused.

Partial views in ASP.NET Core NodeJs Express EJS Layouts and Partials How to use Partials in Rails

As the unit of abstraction in Astro is the component (not partials), and as I understand it the motivation for this RFC is for endpoints, perhaps Fragment Pages or Page Fragments is a better feature name?

PeterDraex commented 10 months ago

The section Including head content but not doctype lists a cons of injecting scripts/styles directly mid-page and concludes that it's never the right approach to include scripts/styles in the partial.

In my case, I am working with an old PHP site with spaghetti code, which I'd like to gradually transition to Astro. During the transition, my plan is to have the Astro partials injected in the HTML. Developing code to manage both the PHP assets and Astro assets together is somewhere between not-worth-it and impossible. On the other hand, I can make sure I develop the Astro components in a manner that repeated injection of styles/scripts would not be an issue. If this RFC goes ahead as-is, I'll have to keep setting partial = false and stripping doctype. Unless you guys see any better way to do the transition than server-side injection of Astro code?

As there are many use cases for partials, it might be good to have the option to configure separately:

What do you think?

matthewp commented 10 months ago

@PeterDraex I'd recommend stripping out the doctype using middleware for your case. It should be very simple to write. We don't want a feature that immediate breaks down once you use a partial more than once.

matthewp commented 10 months ago

@lloydjatkinson thanks for the reference! We original did call it fragments but switched to partials after some discussion. Your points are very valid! Going to discuss it with the core team and see what they think.

philipschm1tt commented 10 months ago

Just wanted to comment I find the name of the feature unclear (though I understand how the name was arrived at). Partials is already a fairly well used terminology to describe reusable templates or "views" particularly in MVC-like frameworks. These partials have little in common with this RFC. Essentially partials in this context are components.

@lloydjatkinson thanks for the reference! We original did call it fragments but switched to partials after some discussion. Your points are very valid! Going to discuss it with the core team and see what they think.

I would prefer "partials" over "fragments" in this case.

"Partials" is not uncommon (also used for things like SCSS) and not strictly about re-using – just says that it's only a part of the markup. That usage is not far off from some of the linked examples: the MVC frameworks often amount to parts of only the markup – the content of an Astro partial and something like a Java JSP "partial" would not be big.

By contrast, "fragment" is increasingly popular in the micro-frontend context (see https://engineering.zalando.com/posts/2018/12/front-end-micro-services.html or https://blog.cloudflare.com/better-micro-frontends/ ). Astro does not support that approach. The word refers more to the "outcome" of a piece of the rendered site, which also "implies" the behavior within that "fragment" and not just the markup.

An alternative term that would work: "html fragment". It's precise: only HTML. Seems fine to me, too.

matthewp commented 10 months ago

We are going to stick with partials, but title it as Page Partials in our documentation and communication, to specify that this is about partials that are accessible via a URL. Naming is hard and there's no perfect name. In the future if this seems like a mistake we can always rename, so this is not a blocking sort of decision.


This RFC has gone through the final comment period, I appreciate everyone's feedback!