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

feat: simplify and split models/definition/filters files #30

Merged
merged 17 commits into from
Aug 27, 2024

Conversation

buuhuu
Copy link
Collaborator

@buuhuu buuhuu commented Jul 19, 2024

  • simplify model files by removing any attributes of the fields that are not strictly required (valueType, value, ...)
  • rename properties of default content models to adhere to block naming conventions (field collapsing)
  • split models/definitions/filters into partials
  • release the used merge-json-cli to npmjs.org

Each block now has a _component.json partial with a json object that contains definitions, models and filters. Each of them is an array as a block may expose 0..n of either of them. The default content model partials are now stored in models/ as well as the partials for component-models.json, component-definition.json and component-filters.json. The file name format with the leading _ was inspired by sass partials (files that are meant to be imported into other files).

Besides that this commit adds husky pre-commit hook support, with a simple hook that merges the models/definition/filters files if any of the partials changed.

Copy link

aem-code-sync bot commented Jul 19, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

blocks/cards/component.json Outdated Show resolved Hide resolved
models/_component-definition.json Show resolved Hide resolved
@aem-code-sync aem-code-sync bot temporarily deployed to split-ue-json-files July 24, 2024 04:32 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to split-ue-json-files July 24, 2024 04:34 Inactive
@buuhuu buuhuu marked this pull request as ready for review July 24, 2024 04:39
@aem-code-sync aem-code-sync bot temporarily deployed to split-ue-json-files July 24, 2024 04:44 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to split-ue-json-files July 24, 2024 10:12 Inactive
@jckautzmann
Copy link
Collaborator

A few suggestions:

  • I’d rename blocks/cards/_component.json to blocks/cards/_cards.json to be consistent with models/_button.json
  • I’d rename component-definition.json and _component-definition.json to make it plural: component-definitions.json and _component-definitions.json to make it consistent with component-filters and component models

@buuhuu
Copy link
Collaborator Author

buuhuu commented Jul 25, 2024

@jckautzmann

I’d rename blocks/cards/_component.json to blocks/cards/_cards.json to be consistent with models/_button.json

Very good suggestion as this allows

  • (a) to use autocomplete search and find the .json by name. Currently if you search for components you find a lot.
  • and (b) allows to split it even further per block like _cards.json and _card.json

I’d rename component-definition.json and _component-definition.json
that would be a breaking change which I would not want to do now. I agree though.

@aem-code-sync aem-code-sync bot temporarily deployed to split-ue-json-files July 25, 2024 20:29 Inactive
@clementepereyra
Copy link

Hi all,
Any ETA when this will be merged? I'm about to migrate a project and would like to know if I should wait a little bit, or just move forward with the current approach.
Thanks.

@buuhuu
Copy link
Collaborator Author

buuhuu commented Aug 19, 2024

This will be merged after the next AEM Cloud Services release scheduled for this / next week.

The majority of changes can be used. Unfortunately I made some changes to the default content models (button, title, image) and rename the fields according to our naming conventions so that they can be used in container blocks without any changes. This change is the only thing that requires the AEM CS release. Spitting/merging the files works today.

@aem-code-sync aem-code-sync bot temporarily deployed to split-ue-json-files August 27, 2024 04:42 Inactive
@buuhuu buuhuu merged commit 986b77f into main Aug 27, 2024
2 of 3 checks passed
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.

5 participants