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

Proposed v0.6 API and structural changes #191

Closed
jimafisk opened this issue May 31, 2022 · 6 comments
Closed

Proposed v0.6 API and structural changes #191

jimafisk opened this issue May 31, 2022 · 6 comments
Labels
breaking Change that's not backwards compatible with previous versions

Comments

@jimafisk
Copy link
Member

We currently hold built javascript assets in a folder called spa, should we rename this folder layouts to better match the initial project files? Name conflicts with user defined routes / static HTML?

Ejected files get put into a folder in the project root called ejected, maybe a better name for this would be core. Would need to update the folder name for the built site as well.

Currently we're performing EjectCopy separately from the Client build process where we prep ejected svelte files that need to get compiled. Seems redundant to walk these files twice, eject_copy should probably just be a func call during the Client build when non .svelte files are found. This would speed up the build process.

I placed the new --minify processing inside of Gopack for now. I didn't want to insert it before Gopack ran because it could screw up the regexp needed to update paths for ESM. Made me think the way we're running Gopack in general is redundant: we're re-reading files already written to the FS during the Client build, updating the content and re-writing them back to the filesystem. We could probably just do this when the files are still []byte after being compiled initially, before they are written to the filesystem. We could do the minification there as well. This would speed up the build process.

@jimafisk jimafisk added the breaking Change that's not backwards compatible with previous versions label May 31, 2022
@jimafisk
Copy link
Member Author

Applied the breaking label just in case you've ejected files that you're customizing. Otherwise, we could probably change these names / structures and most existing sites will continue working fine.

@jimafisk
Copy link
Member Author

jimafisk commented Jun 2, 2022

We should think about reworking pagination as well. No need to create a completely new content object for each pager. We could change the pager content key (tracks the current page) to just be a pages key (tracks total number of paged output). Then in the router if pages is greater than 1, we'd just create a route for each. We might still have to SSR the component multiple times because a layout could be dependent on the current page value.

@jimafisk
Copy link
Member Author

jimafisk commented Jul 16, 2022

I'm planning on making the default pager value inside content objects null (as opposed to 1 like it is now). This logically makes more sense if a pager is not being used and also makes it easier to filter out duplicate items create via pagination when creating lists. For example, if you had a thing type:

const things = allContent.filter(content => content.type === "thing" && !content.pager);

This is a breaking change, because if you were simply setting the current page like I've mentioned before:

$: currentPage = content.pager;

You now need to set the default page yourself:

$: currentPage = content.pager ? content.pager : 1;

I need this for a project I'm working on, so I might sneak it in before the v0.6.0 release, sorry 😬

jimafisk added a commit that referenced this issue Jul 16, 2022
@jimafisk
Copy link
Member Author

The null defaults for pagination are included in v0.5.10. Other items on this list are still outstanding.

@jimafisk
Copy link
Member Author

jimafisk commented Jan 8, 2023

I've been working on this. The structural changes to the built app will be:

  • public/spa/ejected/ => public/spa/core/
  • The following generated files should be grouped into a public/spa/generated/ folder:
    • public/spa/ejected/content.js
    • public/spa/ejected/layouts.js
    • public/spa/ejected/defaults.js
    • public/spa/ejected/schemas.js
    • public/spa/ejected/component_defaults.js
    • public/spa/ejected/component_schemas.js
    • public/spa/ejected/cms/media.js (formerly assets.js)
    • public/spa/ejected/env.js

We probably want to move all compiled layouts into a public/spa/layouts folder instead of scattering them in public/spa like:

  • public/spa/content/
  • public/spa/global/
  • public/spa/components/
  • public/spa/helpers/

Should we hash / fingerprint the /public/spa folder? Would be nice to SRI the content, but might be sufficient for now to generate a random string on each build for this folder name and provide a env variable to use in your <head>. This essentially breaks the cache on subsequent builds (also avoids conflicts if user wants an public/spa/index.html page).

@jimafisk
Copy link
Member Author

jimafisk commented Jan 9, 2023

Most of these structural changes have been released in v0.6.0.

The client is still uploaded into an public/spa/ folder for now, although that may change in a future v0.6.x release (sorry I realize this shouldn't be done in a patch release).

A new issue has been created for build optimizations: #255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Change that's not backwards compatible with previous versions
Projects
None yet
Development

No branches or pull requests

1 participant