ampproject / ampproject/amp.dev
Refactor and break up samplesBuilder.js
- Dominant language
- HTML
- Stars
- 600
- Forks
- 675
- PR merge metrics
- No merged PRs in 30d
Description
### Problem
[samplesBuilder.js](https://github.com/ampproject/amp.dev/blob/future/platform/lib/build/samplesBuilder.js) is an over 700 lines long mess. Its early purpose was to act as the glue between what has been ampbyexample.com before and amp.dev's page builder [Grow](https://grow.io/). To do so it creates three separate files from each sample file which are then consumed by Grow to build dedicated pages for the [sample documentation](https://amp.dev/documentation/examples/components/amp-accordion/?format=websites), [sample preview](https://amp.dev/documentation/examples/components/amp-accordion/preview/?format=websites) and [source/embed files](https://preview.amp.dev/documentation/examples/components/amp-accordion).
Since its first implementation, some things changed: we added support for transforming examples in all of the four AMP formats websites, stories, ads, and e-mail if the relevant runtime supports the showcased features. That's an awesome feature to have though added some more complexity to samplesBuilder.js and makes it a little harder to understand what's going on.
Another thing that changed is that the underlying ampyexample.com parser logic which creates a consumable, structured object from the [sample source file](https://github.com/ampproject/amp.dev/blob/future/examples/source/1.components/amp-accordion.html) moved from the deprecated ampbyexample.com repository into the amp.dev codebase. This allows us to move a lot of the logic that relates to the document and its content back to [ExampleFile.js](https://github.com/ampproject/amp.dev/blob/future/platform/lib/samples/ExampleFile.js) and start using the abe.com parser's API again instead of passing around anonymous objects in samplesBuilder.js.
To top it all off samplesBuilder.js currently is blocking and processes one file after another instead of doing it parallelly.
### Solution
None specific yet. What I'd like to gather in this issue are other pain points like the one from #3416. Ultimately I imagine an updated sampleBuilder.js to work like the new `ComponentReferenceImporter` from #3344: really only have it to read the source files and then create dedicated objects for each of the targets (documentation, preview, source) which aren't as tightly coupled as it's currently the case and therefore are finally testable - which with you'd have a hard time currently.
### Alternatives
This is not urgent. But if it's on me there is no real alternative as in its current state it's really hard to extend or maintain without breaking anything.
Contributor guide
Research direction
Start with platform/lib/build/samplesBuilder.js and platform/lib/samples/ExampleFile.js to understand the current sample parsing and target-file flow. Compare the design with ComponentReferenceImporter from #3344 and review #3416 for related pain points; done should mean the refactored responsibilities and testable target objects are clearly defined.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- build-system
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100