Skip to content

I2I: src/core directory with no runtime dependencies #32693

@rcebulko

Description

@rcebulko

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

  • Create /src/shared directory
  • 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
  • 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    INTENT TO IMPLEMENTProposes implementation of a significant new feature. https://bit.ly/amp-contribute-codeStaleInactive for one year or moreWG: bento

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions