toastdotdev / toast

The best place to stack your JAM. Toast is a Jamstack framework
153 stars 13 forks source link

page wrapper API changes #29

Closed ChristopherBiscardi closed 3 years ago

ChristopherBiscardi commented 3 years ago

As a developer using Toast, I want to be able to achieve the following:

A notable point is that in other frameworks, I've had the frustration of working with different processing pipelines for createPage and src/pages analogues and I'd like to avoid that here.

If we consider src/pages to be "createPage", then it is sugar on top of createPage that leads us toward modularized apis (aka themes, or whatever you want to call them). By making src/pages feed into createPage people will need the ability to override the page wrapper for specific pages in src/pages. While you already have control over this if you use createPage directly, src/pages CreatePage structs will need to be overridden or shadowable through user-defined mechanisms.

This leads to a priority facade where src/pages is considered "less truthful" than user createPage calls. If there is a facade here, then we should be able to trace all changes to the data over time and show that to the user. "X was modified by Y at Z" effectively creating an event sourced system with a full audit log?


for createPage it makes sense to add a pageWrapper field I think, resulting in the following API:

createPage({
  slug: "/somewhere",
  component: <source-code-as-a-string>,
  data: {...},
  wrapper: <source-code-as-a-string>
})

This allows wrappers to come from remote sources, which is nice, but makes working with local files a bit more painful. Pulling in src/page-wrapper.js requires an fs.readFile and this is something users are likely to get wrong (ex: readFile on every createPage call).

To this end, there should likely be sugar on top of this API that accepts filepaths instead of component strings. Then we can get the logic for reading files in right in the framework, making it easier to build performant data pipelines for users.

createPage({
  slug: "/somewhere",
  component: <source-code-as-a-string> | <filepath>,
  data: {...},
  wrapper: <source-code-as-a-string> | <filepath> | false
})

I think this API is OK, but potentially has implications for determining whether something is a filepath or a component string. There are crates like Url that can assist with this, but if the URL is malformatted and it registers as a component, that results in confusing error messages for the user (because Toast won't know and has to default to something). It might be better to allow two fields or a discriminator so the user can indicate their intent, which results in better error messages specific to the intended behavior.

{
  value: <source-code-as-string> | <filepath-relative-to-src-dir>,
  mode: 'filepath' | 'source'
}
createPage({
  slug: "/somewhere",
  component: {
    value: 'page-wrapper.js',
    mode: 'filepath',
  },
  data: {...},
  wrapper:  {
    value: <some-component-source-code>,
    mode: 'source',
  },
})

This could theoretically also be extended in the future:

{
  value: <source-code-as-string> | <filepath-relative-to-src-dir>,
  mode: 'filepath' | 'source',
  type: 'default' | 'vue-sfc' | 'something-else'
}

src/pages

So now that the createPage API is worked out, the src/pages needs an "override"-ability. I think this works out if successive createPage calls merge over the top of existing values and user createPage calls "outrank" src/pages.

so src/pages/index.js is effectively:

createPage({
  slug: '/',
  component: {
    value: 'pages/index.js',
    type: 'filepath'
  }
})

and then users can do this to overlay on top.

createPage({
  slug: '/',
  data: {...},
  wrapper: <some-code> | false
})

That brings up a different issue, in that setData is now basically just an "extra function" because createPage can be used with this, which is exactly what setData does.

createPage({
  slug: '/',
  data: {...}
})

We have to handle data for pages anyway, so this doesn't introduce any extra complexity that wouldn't already be there, but it does mean that createPage is a weird name for an overloaded function that can do multiple things.

ChristopherBiscardi commented 3 years ago

the new API is setDataForSlug and is designed as described above with the caveat that we moved slug out to the first positional argument as it is always required anyway.

setDataForSlug("/", {
  component: {},
  data: {},
  wrapper: {}
})