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

Frontend code extraction & folder restructuring #10790

Closed
6 of 8 tasks
naz opened this issue Jun 18, 2019 · 0 comments
Closed
6 of 8 tasks

Frontend code extraction & folder restructuring #10790

naz opened this issue Jun 18, 2019 · 0 comments
Labels
later [triage] Things we intend to work but are not immediate priority server / core Issues relating to the server or core of Ghost

Comments

@naz
Copy link
Contributor

naz commented Jun 18, 2019

This issue is a placeholder for planned changes that reduce coupling of handlebars frontend with the rest of codebase.


Goal

Ghost is moving toward reducing coupling and colocation of dynamic frontend related code (handlebars helpers, themes, etc.) from strictly backend code (APIs, internal services, etc.)

Code structure

The decision was made to start with the extraction of new /frontend folder which will be located in /core along with /client and /server:

core
├── client
├── frontend
└── server

First elements extracted out of /server folder will be: apps, helpers, services (URL, themes, routing, etc.).

Next steps will be the extraction of frontend specific controllers (themes, redirects, partially settings) and creation of an "adapter" that will allow optionally swap or disable default handlebars driven frontend.

Methods from extracted controllers will be moved into according services in /core/frontend/services as follows:

settings.download 	-> routing.settings.get
settings.upload 	-> routing.settings.setFromFilePath

redirects.download	-> redirects.settings.get
redirects.uplaod	-> redirects.settings.setFromFilePath

themes.download		-> themes.settings.get
themes.activate		-> themes.settings.setFromFilePath

Rought Plan

Code structure:

Test structure:

  • To be discussed and filled out

Future improvements

The services extracted out of controller code are using HTTP specific error codes which don't fit on 'service layer' logic as it doesn't have to know anything about communication protocol (it's controllers role to do that). This is tangential to the current issue, but something to be awere/improve upon with next iterations 👍

Related

Server folder restructuring - #9178
Coupling reduction through code extraction into Ghsot-SDK - #10773 , #10618
URL Service decoupling - #10360

@naz naz added server / core Issues relating to the server or core of Ghost refactoring labels Jun 18, 2019
@naz naz mentioned this issue Jun 18, 2019
1 task
naz added a commit that referenced this issue Jun 19, 2019
refs #10790

- Moved /core/apps into core/frontend
- Moved /core/server/helpers to /core/frontend/helpers along with /core/server/services/themes
- Changed helper location in overrides
- Moved /core/server/services/routing to /core/frontend/services
- Moved /core/server/services/url to /core/frontend/services
- Moved /core/server/data/meta to /core/frontend/meta
- Moved /core/server/services/rss to /core/frontend/services
- Moved /core/server/data/xml to /core/frontend/services
naz added a commit that referenced this issue Jun 19, 2019
refs #10790

- The name should be themeService as everywhere else in the codebase
naz added a commit that referenced this issue Jun 21, 2019
refs #10790

- The code was moved out of controllers to reduce the number of coupling points between the API controllers and "frontend" services
- A nice side effect of this move is a decreased amount of code that will need to be maintained and reusability between existing controllers
- Calling just a few methods from frontend services on API level makes it easier to abstract fronted away from API
naz added a commit that referenced this issue Jun 24, 2019
refs #10790

- Reference to method previously used to validate wasn't updated during refactoring done in be27db4
naz added a commit that referenced this issue Jun 24, 2019
refs #10790

- The error was happening due to wrong reference to the site app from themes service
- The issue was introduced in df7e64f
naz added a commit that referenced this issue Jun 25, 2019
refs #10790
refs #9528

- The settings service was designed to handle more settings then just routing, but till this day there wasn't anything else added. As routes.yaml is only being used by frontend router so conceptually it fits better to have this code in frontend, so that it doesn't have to reach out to server
- The code left in server settings is the one that interacts with the database `settings` table and only partially provides information to frontend. That part is known as 'settings cache' and will be accessed through API controllers.
naz added a commit that referenced this issue Jul 1, 2019
refs #10790

- Extracted 'setFromZip' method into themes services
- Extracted 'activate' method
- Extracted 'destroy' method
- Extracted 'download' method
- The method name here tries to follow 'setFrom...` convention we've agreed upon. So, in this case, we have get() which returns JSON response and getZip() which returns a file
naz added a commit to naz/Ghost that referenced this issue Jul 8, 2019
refs TryGhost#10790

- The TODO was waiting for 2 years and today is the day to cross it out
- "Reduced the amount of things we expose to the outside world"
- "Made this a nice clean sensible API we can all understand!" - by @ErisDS
naz added a commit that referenced this issue Jul 9, 2019
refs #10790

- Following TODO in theme index file was waiting for 2 years, and today is the day to cross it out:
- "Reduced the amount of things we expose to the outside world"
- "Made this a nice clean sensible API we can all understand!" - by @ErisDS
- Cleaned exposed methods from themes module
- Removed unused storage getter
- Removed list method
- Removed validate method
- Renamed Storage to ThemeStorage
  - Named the file the same way the class defined inside of it is named
  - Naming was conflicting with coming rename of  `settings` -> `storage`
- Renamed theme settings to storage
@ErisDS ErisDS added the later [triage] Things we intend to work but are not immediate priority label Aug 21, 2019
@ErisDS ErisDS closed this as completed Aug 21, 2019
naz added a commit to naz/Ghost that referenced this issue Jul 15, 2021
refs https://github.com/TryGhost/Team/issues/527
refs TryGhost#10790

- Frontent has to have as few as possible coupling points with the Ghost Server API. By design that point has been a "proxy.api" property that will become more and more constraint in the future based to limit the surface of frontend interaction with servers's API
- Removing `.../server/api` requires in favor of using a proxy decreases direct coupling
naz added a commit that referenced this issue Jul 19, 2021
refs https://github.com/TryGhost/Team/issues/527
refs #10790

- Frontent has to have as few as possible coupling points with the Ghost Server API. By design that point has been a "proxy.api" property that will become more and more constraint in the future based to limit the surface of frontend interaction with servers's API
- Removing `.../server/api` requires in favor of using a proxy decreases direct coupling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
later [triage] Things we intend to work but are not immediate priority server / core Issues relating to the server or core of Ghost
Projects
None yet
Development

No branches or pull requests

2 participants