Skip to content

refactor(astro): add internal entry points for test#16473

Open
ocavue wants to merge 3 commits intowithastro:mainfrom
ocavue-forks:ocavue-test-cleanup3
Open

refactor(astro): add internal entry points for test#16473
ocavue wants to merge 3 commits intowithastro:mainfrom
ocavue-forks:ocavue-test-cleanup3

Conversation

@ocavue
Copy link
Copy Markdown
Contributor

@ocavue ocavue commented Apr 23, 2026

Changes

In other packages in this repo, we sometimes want to import the internal API from astro. Previously, we relied on a long relative path, which is not only ugly but also doesn't function well for typescript project references.

This PR introduces astro/_internal/... entry points that export some internal APIs for testing purposes. Here are some examples:

// packages/integrations/cloudflare/test/ssr-deps.test.ts
-import { AstroLogger } from '../../../astro/dist/core/logger/core.js';
+import { AstroLogger } from 'astro/_internal/logger';
// packages/integrations/alpinejs/test/test-utils.ts
-import { loadFixture } from '../../../astro/test/test-utils.js';
+import { loadFixture } from 'astro/_internal/test/test-utils';

We don't want these astro/_internal/... entry points to be published to the NPM registry. I use PNPM's publishConfig feature to override the exports field when publishing.

Testing

Since we now have both exports and publishConfig.exports in packages/astro/package.json, I've added a test in packages/astro/test/exports.test.ts to ensure that these two objects stay in sync with each other. This prevents the case that someone adds a new entry point in exports but forgets to add it in publishConfig.exports.

Docs

No public API change so no changeset file.

CONTRIBUTING.md is updated.


Part of #16241

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 23, 2026

⚠️ No Changeset found

Latest commit: 3c8f4fd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: react Related to React (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) docs pr labels Apr 23, 2026
@ocavue ocavue force-pushed the ocavue-test-cleanup3 branch from bb7b084 to bdb23b3 Compare April 24, 2026 00:04
@ocavue ocavue force-pushed the ocavue-test-cleanup3 branch from bdb23b3 to 0cfd80d Compare April 24, 2026 00:13
@github-actions github-actions Bot removed the docs pr label Apr 24, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 24, 2026

Merging this PR will not alter performance

✅ 18 untouched benchmarks


Comparing ocavue-forks:ocavue-test-cleanup3 (bdb23b3) with main (b2d8eb3)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (f56bb3f) during the generation of this report, so b2d8eb3 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@ocavue ocavue changed the title WIP: clean up ts tests (3/n) refactor(astro): add internal entry points for test Apr 24, 2026
@ocavue ocavue marked this pull request as ready for review April 24, 2026 00:33
Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bigger change that might require updating the contribution guidelines. Can you do that please?

@ocavue
Copy link
Copy Markdown
Contributor Author

ocavue commented Apr 24, 2026

Will do

@ocavue ocavue requested a review from ematipico April 24, 2026 17:50
Copy link
Copy Markdown
Contributor

@Eveeifyeve Eveeifyeve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@florian-lefebvre
Copy link
Copy Markdown
Member

For test related things like loadFixture, could we instead have an internal workspace non published package? That would make more sense to me.

For AstroLogger I'd say it depends. Feels like a may be public soon with Ema's work

@ocavue
Copy link
Copy Markdown
Contributor Author

ocavue commented Apr 25, 2026

For test related things like loadFixture, could we instead have an internal workspace non published package

There are multiple approaches to doing it, each with different trade-offs. Here are three examples (I can think of more, but they are similar).

  1. (The current PR) Add _internal/... entry points to the astro package

    Pros: Less diff compared to all other approaches below

    Cons: Need to use PNPM publishConfig to get rid of these entry points during publishing

  2. Add a new packages/test-utils/package.json package (named astro-test-utils) and move all test utils into it.

    Pros:

    • No need for publishConfig compared to approach 1.
    • Split the astro package's own tests and shared test utils that can be used in multiple packages.

    Cons:

    • Circular dependency which PNPM would warn about. The dependency relations would be:
      • astro-test-utils depends on astro
      • astro also depends on astro-test-utils because packages/astro/test/*.test.ts files need to import it.
  3. Based on 2, but also add a new packages/astro/test/package.json (named astro-test). The dependency relations would be:

    • astro-test-utils depends on astro
    • astro-test depends on astro-test-utils and astro
    • astro depends on no one
    • @astrojs/* depends on astro-test-utils as a dev dependency.

    Pros:

    • No circular dependency compared to approach 2.

    Cons:

    • This can only resolve test utils. It cannot resolve internal APIs like "./_internal/assets" and "./_internal/loggers", which are not public yet today.
    • We might need to move the test script to a different package.json file, which could confuse devs.

I chose 1 in this PR because it's the one with the fewest git diffs, doesn't require any prerequisites, and fits the existing file/package structure, so it's the easiest one to land. But I'm also open to other approaches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) pkg: react Related to React (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants