Service definition export format #2397
Conversation
Generated by 🚫 dangerJS |
There was a problem hiding this comment.
I've had a go at reviewing this commenting on the "how", but could you also provide a bit more info on the "why". i.e: what are the problems with badge-examples.json that service-definitions.yml is solving (in terms of data structure rather than code that generates it). Having a slightly clearer idea of the high-level objective and plan for future work here would be useful.
|
I significantly expanded on the why in the top comment. Does that make sense? |
|
Yeah that extra info makes sense. Thanks. One other thing I've thought about is performance. I might be wrong on this because I have not tried benchmarking it, but I've got a hunch that the way we're working with the badge examples data structure in https://github.com/badges/shields/blob/master/frontend/lib/prepare-examples.js is one of the things that makes the front end laggy on mobile devices. We certainly spend more time iterating over that data structure than we need to. Maybe its a red herring, but while we're redesigning this data structure, have you given any thought to whether this is more or less efficient to group/search? |
|
Yea, it would be good to improve the frontend performance. I haven't dug into it that much either. My instinct is it's related to shipping too much JavaScript, DOM manipulation, or requesting all the SVGs. Dropping Next would probably have a big impact. We could also inline the SVGs. That should be easy to do for the static examples. I doubt string matching or data-structure manipulation is what's making the frontend slow, though if we needed to group the examples for performance reasons, we could do it once when they're first loaded. |
- The goal of this PR is:
- Consume the new service-definition format. (#2397)
- Make the frontend more readable.
- Behavior changes:
- I changed the **Image** field in the markup modal to show only the path.
- I added another click-to-select field below that shows the complete URL.
- This made it easier to suppress the live badge preview while it contains placeholders like `:user` or `:gem`, a minor tweak discussed at #2427 (comment).
- The search box now searches all categories, regardless of the current page. (This is an improvement, I would say.)
- I did not deliberately address performance, though I ripped out a bunch of anonymous functions and avoided re-filtering all the examples by category on every render, which I expect will not hurt. I haven't really tested this on a mobile connection and it'd be worth doing that.
- It would be great to have some tests of the components, though getting started with that seemed like a big project and I did not want to make this any larger than it already is.
It's a medium-sized refactor:
1. Replace `BadgeExamples`, `Category` and `Badge` component with a completely rewritten `BadgeExamples` component which renders a table of badges, and `CategoryHeading` and `CategoryHeadings` components.
2. Refactor `ExamplesPage` and `SearchResults` components into a new `Main` component.
3. Rewrite the data flow for `MarkupModal`. Rather than rely on unmounting and remounting the component to copy the badge URL into state, employ the `getDerivedStateFromProps` lifecycle method.
4. Remove `prepareExamples` and `all-badge-examples`.
5. Rewrite the `$suggest` schema to harmonize with the service definition format. It's not backward-compatible which means at deploy time there probably will be 10–20 minutes of downtime on that feature, between the first server deploy and the final gh-pages deploy. 🤷♂️ (We could leave the old version in place if it seems worth it.)
6. Added two new functions in `make-badge-url` with tests. I removed _most_ of the uses of the old functions, but there are some in parts of the frontend I didn't touch like the static and dynamic badge generators, and again I didn't want to make this any larger than it already is.
7. Fix a couple bugs in the service-definition export.
Three main goals:
a. Show form fields and automatically assemble badge URLs (Add Username and Repo Name to Shield Creation fields #701)
c. Group together examples for the same service
b. Show deprecated services
examples, thanks to 100% validation. One challenge with frameworks is that when there are typos things fail silently which is pretty unfriendly to developers. The validation should really help with that. (This caught one bug in AUR, though I fixed it in Tweak [AUR] and add static examples #2405 which landed first.)exampleskey in the service classes. I aliasedstaticPreviewtostaticExamplewhich starts this process.The old format:
The new format improves a few things, too:
.svg). (I can imagine dropping the.svgfrom our badge URLs someday, which would make the URLs easier to read and maintain.)Here are a couple snippets:
If you run
npm run defsyou can see the whole thing inservice-definitions.yml. You can also see the schema inservices/service-definitions.js.Though this format is redundant with
badge-examples.json, it’s initially being added in parallel. That's to make this change more manageable to review.After the frontend is modified to consume the new format, the old code (
prepareExamples(),export-badge-examples-cli.js, and friends) will be removed.BaseService.exampleshas its own schema. This adds new functions for validating and transforming the content ofexamples, which has its own schema). These are covered by new tests inbase.spec.js.To harmonize
BaseService.exampleswith the new format, this renames two keys:query->queryParamsstaticExample->staticPreviewThe old names are aliased, though the aliases needn't be in place for long. Especially since
examplesis now fully validated, they can be removed with a search and replace soon after this is merged (like #2341). (We could also go withpreviewinstead ofstaticPreview, which might be even better.)I would say this closes #776, though there is a bit of housekeeping to do, like formalizing the schema and publishing an initial copy to npm.
Blocks the frontend refactor and the follow-on cleanup work.