feat: register custom formats take 2
Replaces #5839 Fixes #5848
Summary
Adds the ability to register custom formats (could be xml, po, binary, json5, etc.).
Also adds the requested tests and docs from the last PR (1dc43db87936fa480d3650972f2bf0a0882da726 + 6d29b239cb36073d4dde8d5bd0d3db87c86eab07)
Test plan
Unit tests for register function + formatters themselves. Then ran manual tests by doing npm pack, installed the .tgz file directly in the project we're using netlify-cms with, then registering some custom formats... and it worked.
Checklist
- [x] I have read the contribution guidelines.
- [x] Code is formatted via running
yarn format. - [x] Tests are passing via running
yarn test. - [x] The status checks are successful (continuous integration). Those can be seen below.
@erezrokah any ideas why cypress is failing? I wasn't able to run them myself and don't have permission to download screenshots.
Looks like our tests started failing with a specific renovate PR https://github.com/netlify/netlify-cms/actions/workflows/nodejs.yml?query=branch%3Amaster.
Looking into it
@erezrokah using yarn test:e2e:dev the can change status on and publish multiple entries test was passing for me locally, so I don't think the failure is related to these changes, unless I'm misunderstanding something?
@erezrokah using
yarn test:e2e:devthecan change status on and publish multiple entriestest was passing for me locally, so I don't think the failure is related to these changes, unless I'm misunderstanding something?
You're correct. I don't think the failure is related to your changes. However I'd prefer to fix the tests before merging this PR.
@erezrokah thank you for helping get green! It says there's one workflow awaiting approval. Anything else I should do to get this over the line?
It says there's one workflow awaiting approval
That's GitHub's crypto mining defense technic :) First committers need approval from a maintainer to run the CI.
I'm trying to think of a way to avoid manual initialization. We could have the custom formats defined as top level item config.yml and use that to validate, WDYT?
It will also make it clear what's supported
It says there's one workflow awaiting approval
That's GitHub's crypto mining defense technic :) First committers need approval from a maintainer to run the CI.
I'm trying to think of a way to avoid manual initialization. We could have the custom formats defined as top level item
config.ymland use that to validate, WDYT?It will also make it clear what's supported
I'm not sure - wouldn't some custom javascript have to run to register the formatter implementation? Where would netlify-cms know where to find that javascript if there's no manual initialization. Also - I don't feel very comfortable adding non-manual initialization support since the team have been using it since day one.
Would you be open to it going in without, then if there are users requesting custom formats without wanting to manually initialize, a separate piece of work could be done to support that use case too? (Selfishly - that wouldn't help my team and we would love to be able to use this now!)
Thanks for your input @mmkal, what concerns me is that the CMS default is not to have manual initialization so requiring it here breaks that. Also other register* methods don't require it.
Another option is to drop the static config level schema validation and only validate dynamically when trying to retrieve the format. We do the latter for custom widgets and display an "Unknown" widget when that happens.
WDYT?
Thanks for your input @mmkal, what concerns me is that the CMS default is not to have manual initialization so requiring it here breaks that. Also other
register*methods don't require it.
I'm not sure I agree it breaks that. The default setup still doesn't require manual initialisation - it's only required if you want to start pretty seriously customising the behaviour by adding extra file formats. Having said that, I can't see why it needs to be any different from registering a custom widget.
Another option is to drop the static config level schema validation and only validate dynamically when trying to retrieve the format. We do the latter for custom widgets and display an "Unknown" widget when that happens.
WDYT?
I think this is already happening thanks to this change:
If someone tries to use an un-registered widget, there will just be an error thrown. So maybe this is already meeting the requirements? Is there a simple way to test this with a non-manual-initialized setup?
@erezrokah just tested this and it actually doesn't require manual initialization. It works similarly to the other CMS.register... methods.
It looks like this if you try to use a non-existent formatter:
So to address your two comments:
- Manual initialization: not required.
- Drop static validation: done. We now only validate that the prop is a string.
So, anything else needed for this to go in?? 🙏
@erezrokah bump - anything I can do to get this in?
Ping @erezrokah - CC @bytrangle in case Erez is no longer working on netlify-cms?
pinging @krider2010
@mmkal Thank you for the time you've put into this PR. I will find time to review this soon.
Just a heads up I'm working on being added to the maintainer group for review reasons.
@bytrangle @krider2010 @erezrokah fixed the prettier problem - hopefully ready now but won't know til the build is approved! 🙏
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
I still want to merge this!
Deploy Preview for decap-www ready!
| Name | Link |
|---|---|
| Latest commit | aed01d0ef5aac25987499f2069e23a0d6c7d4b52 |
| Latest deploy log | https://app.netlify.com/sites/decap-www/deploys/6488a17230778a00081b11b3 |
| Deploy Preview | https://deploy-preview-6205--decap-www.netlify.app/docs/beta-features |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
@martinjagodic the only thing failing (after a year and a half of "Update from main and cross fingers") is Markdown widget breaks pressing enter:
I really don't think it's related - could we merge this as is? I've updated to incorporate all the netlify-cms-* to decap-cms-* package changes.