ampproject / ampproject/amphtml

I2I: `src/core` directory with no runtime dependencies

Open
#32,693 7 comments 0 reactions 0 assignees View on GitHub
INTENT TO IMPLEMENT Stale WG: bento
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

## Overview

The main `amphtml/src` directory holds over 100 JS files (almost 300 if you count subdirectory contents). Some of these files are isolated utilities like date parsing, string manipulations, etc. Others seem like small utilities, but may actually depend on large chunks of the runtime and a whole web of dependencies

**Example of issue**: At the time of this writing, `src/preact/base-element` imports `devAssert`, which in turn initializes logging constructors, two different loggers, binds a global error handler, pulls in the AMP config and runtime version, and `getMode` helpers reading from AMP global variables. Suddenly, because of this one import, every Bento component depends directly on all of this AMP boilerplate. This is an extremely easy oversight for any developer.
**Edit:** There are other dependencies of `src/preact/base-element` that have transitive dependencies on assertions/logging; it is not just this import

> Note: This example and others relying on `devAssert` are easily resolved with the pure assertion helpers in `src/pure-assert.js`, but the issue remains.

**AMP** components can rely on and use any of these files. **Preact**/**Bento** components cannot, or they'll pull in whole chunks of the AMP dependency tree. As more Bento components are written, more shared helpers are needed, and it becomes harder to pull only "pure" helpers.

I propose creating a dedicated directory (ex. `amphtml/src/shared`) for these files. New shared helpers would go here. Existing helpers could be moved gradually as they are needed. ESLint rules could be added to block importing from non-shared modules so devs get a warning at dev-time rather than by seeing the bundle size after compilation.

## Suggested Approach

* Dedicated `src/shared/` directory
* Anything in this directory can import from `./*` and remain shared
* Anything in `src/preact` can import from `src/shared` or `src/preact` without pulling in AMP dependencies
* AMP and Bento code can import from `./shared`, including runtime helpers
* This directory can become the Bento/Preact shared module on NPM
* We don't want to duplicate the src-dependency-web
* Promote small helper modules with clear functions
* Create `shared` directory requiring explicit add-in, instead of "opt-out" `runtime` directory
* No bulk-moving existing helpers into `shared`; only move helpers as they are used
* ESLint Rule
* Disallow any Bento or Preact code from importing non-shared `src/` files
* If possible, disallow anything with side effects

**TL;DR - Directory of helpers that can be included with Preact/Bento packages and have nothing to do with AMP**

### Steps

- [x] Create `/src/shared` directory
- [x] Write ESLint rule to assert file X imports only from `/src/shared` and `/src/preact`
* Applies to all files in both of those directories
- [ ] Migrate existing helpers **that are already used in Preact** into `/src/shared`
- [x] Expand support for linter to include opted-in component files
- [ ] Write or find existing ESLint rule to assert shared files are idempotent
* Until then: diligent OWNERS during review

## AlternativesConsidered

* Have a bunch of `pureFooBar` helpers
* Pro: usage is clear and visible inline
* Con: somewhat ugly and verbose, very much AMP-centric not Bento-centric
* No structural changes; just be careful about imports + raise compilation error
* Pro: minimal code changes
* Con: error-prone; developers don't catch dependencies until bundle-size time, if at all

/cc @ampproject/wg-bento @ampproject/wg-performance

Contributor guide

Open the contributing guide

Research direction

Start by reviewing src/preact/base-element, src/pure-assert.js, and the existing src/shared directory and ESLint rule. Trace which helpers already used by Preact still pull in AMP dependencies, then check the related closed pull requests for prior decisions. Done means the remaining targeted helpers are migrated without broad bulk moves and shared-import or idempotence checks are addressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
eslint, javascript
Domain
tooling
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.