Skip to content

feat: Add marko/vite package#42

Merged
markshust merged 2 commits into
marko-php:developfrom
ps-carvalho:feature/marko-vite-package
May 1, 2026
Merged

feat: Add marko/vite package#42
markshust merged 2 commits into
marko-php:developfrom
ps-carvalho:feature/marko-vite-package

Conversation

@ps-carvalho
Copy link
Copy Markdown
Contributor

Summary

This is PR 1/5 in the stacked package-import sequence for Marko Vite + Inertia packages.

Preceding dependency PR: none. This PR starts the stack from upstream/develop.

Adds marko/vite as a monorepo package under packages/vite, registers it in root Composer metadata, and imports the package tests without standalone package tooling files.

Maintainer Feedback Addressed

  • Config-loading failures are loud through package exceptions instead of falling back silently.
  • Standalone tooling files are not imported.
  • Package-local Composer scripts and redundant tooling dev dependencies are removed.
  • marko/env remains a runtime dependency because config/vite.php uses env().

Test Plan

  • composer validate --strict --no-check-lock
  • vendor/bin/pest tests/IntegrationVerificationTest.php tests/PackagingTest.php packages/vite/tests --colors=never
  • composer test -- --colors=never

Note: composer cs:check is currently blocked by an existing monorepo PHPCS path issue: The file "demo/app" does not exist.

@ps-carvalho ps-carvalho changed the title Add marko/vite package feat: Add marko/vite package Apr 27, 2026
@github-actions github-actions Bot added the enhancement New feature or request label Apr 27, 2026
@markshust markshust self-assigned this May 1, 2026
Apply maintainer review feedback before merge:

- Replace silent HTML-comment fallbacks in manifestTags() with
  ViteManifestException so missing/invalid manifests surface loudly
- Drop `final` modifier on ViteConfigurationException (CLAUDE.md: no
  final classes — blocks Preferences extensibility)
- Simplify module.php to a list-style singletons array; enabled defaults
  to true and autowiring covers the binding (BindingRegistry.php:40-48)
- Rewrite README to the slim-pointer format from docs/DOCS-STANDARDS.md
- Reformat composer.json to 4-space indent matching all other packages
- Fix .gitattributes column alignment after /.github
- Add docs/src/content/docs/packages/vite.md per DOCS-STANDARDS:174

All 13 vite tests + 21 integration/packaging tests pass; phpcs and
php-cs-fixer are clean.

Co-Authored-By: Paulo Carvalho <paulofrediani@icloud.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@markshust
Copy link
Copy Markdown
Collaborator

Thanks for the cleanly-scoped PR @ps-carvalho — package shape, conventions, and the additive approach are all spot on. Pushed a follow-up commit (c1f37dd) on top of yours with the maintainer-side polish so this can land:

Loud errors

  • manifestTags() was returning HTML-comment fallbacks (<!-- Vite manifest not found -->, etc.) for missing/invalid manifests, missing entries, and invalid entry files. Per Marko's "loud errors" principle, those now throw a new Marko\Vite\Exceptions\ViteManifestException with helpful context + suggestions. Tests updated to match.

Framework conventions

  • Removed final from ViteConfigurationException (CLAUDE.md: no final classes — blocks Preferences extensibility).
  • Simplified module.php to a list-style singletons array. enabled defaults to true in ModuleManifest, the sequence: after marko/config wasn't needed (DI resolves lazily), and list-style singletons trigger autowiring directly via BindingRegistry::registerModule() so the explicit bindings entry was redundant.
  • Reformatted composer.json to 4-space indent matching every other package.
  • Fixed .gitattributes column alignment.

Docs

  • Rewrote README.md to the slim-pointer format from docs/DOCS-STANDARDS.md.
  • Added docs/src/content/docs/packages/vite.md — every package needs a corresponding docs page (we usually let the doc-updater agent handle this post-merge, but for a clean contributor PR it should ship complete).

All 13 vite tests + 21 packaging/integration tests pass; phpcs and php-cs-fixer are both clean.


Next up: per the package sequence in #17, marko/inertia is the natural follow-up — it depends on marko/vite and is where the bigger Inertia review notes from this comment come into play (real session for flash(), inject SsrTransportInterface instead of new-ing it, use marko/testing fakes). When you're ready, open it as the next stacked PR and I'll run the same review pass.

Appreciate the work 🙏

@markshust markshust merged commit a1589fb into marko-php:develop May 1, 2026
markshust added a commit that referenced this pull request May 1, 2026
markshust added a commit to ps-carvalho/marko that referenced this pull request May 1, 2026
…licy

Updates `.claude/pr-review-process.md` based on lessons from PR marko-php#42 (marko/vite):

- Adds package-PR-specific checks (5a): module.php is optional and minimal,
  composer.json conventions, full new-package file structure, cross-cutting
  updates (issue-template dropdowns, test-count bumps, required docs page
  at docs/src/content/docs/packages/{name}.md)
- Adds explicit "loud errors / no silent fallbacks" guidance
- Adds "no defensive instanceof checks on container resolutions"
- Adds "no pseudo-functionality" review item
- Notes that no PR CI workflow exists; the merge-readiness signal is local
  lint/tests + mergeStateStatus CLEAN
- Adds step 7 for drafting PR comments before posting
- Reaffirms --merge (not --squash) as the merge strategy and explains
  Co-Authored-By trailers go on maintainer fix commits, not the merge
- Expands the pre-merge checklist with package-specific items

Adds a "Reviewing PRs" section to CLAUDE.md that references the doc as
lookup-on-demand (not @ imported) so it loads only when relevant. Expands
the .claude/ configuration list to include all existing files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ps-carvalho pushed a commit to ps-carvalho/marko that referenced this pull request May 1, 2026
…ackagist)

Three related fixes to the release/publish pipeline, all motivated by
problems we hit cutting 0.4.0 and 0.4.1.

bin/release.sh — replace the gh `releases/generate-notes` call with a
deterministic generator that walks `git log PREV_TAG..HEAD` for `#NN`
references and fetches each PR by number. The API matched PRs to the
tag range by their stored `merge_commit_sha`, which can go stale during
rapid concurrent merges (this is what dropped PR marko-php#42 from 0.4.0 notes).
Walking git ourselves sidesteps the SHA-matching dance entirely. Notes
include category bucketing per `.github/release.yml` and a "New
Contributors" section computed via the search API.

.github/workflows/split.yml — make the "Notify Packagist" step self-heal.
On HTTP 404 from `update-package`, call `create-package` to register the
package, then retry the update so the new tag actually gets indexed.
This removes the manual `register-packagist.sh` requirement when adding
a new package: contributors can submit a PR adding `packages/foo/` and
the first release tag will register and publish it without anyone
running anything locally with Packagist credentials. Without this, three
packages added in 0.4.0 (vite, debugbar, inertia) had failing split
runs because they were never registered on Packagist.

CHANGELOG.md — backfill the 0.1.1 entry. The auto-generator dropped two
unlabeled PRs (marko-php#2, marko-php#5) from the original GitHub Release; restoring them
under "Maintenance" since both were release-tooling work.

.claude/release-process.md — document both changes: explain the
deterministic generator, note that contributors no longer need
Packagist credentials to add a package, and clarify the new failure
mode (a PR missing from notes can only happen if its commit message
omits the `#NN` reference).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants