Enable building out nuxt modules#5
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds npm/package-registry Copier variables and derives Nuxt module naming in context; introduces a Nuxt module scaffold (module, plugin, playground), devcontainer, ESLint/Vitest configs, CI/publish workflows and helper script, Jinja validation tests, test fixtures/data, and replaces some pre-commit hooks and ty.toml excludes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Script as get_npm_version.py
participant CI as Build/Test Jobs
participant Staging as Staging Registry (npm / CodeArtifact)
participant Primary as Primary Registry (npm / CodeArtifact)
participant Git as Git remote
GH->>Script: extract package version
GH->>CI: run lint, tests, build
CI-->>GH: upload artifacts
GH->>Staging: authenticate (OIDC or token) and publish --tag rc
GH->>Staging: validate install / wait for availability
alt publish_to_primary enabled
GH->>Script: confirm tag not present
Script->>Git: create & push tag
GH->>Primary: authenticate and publish (promote)
GH->>Primary: validate install / wait for availability
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extensions/context.py`:
- Around line 125-130: The current splitting uses bare.split("-") which fails
for underscore-delimited package names; update the tokenization so parts =
re.split(r"[-_]", bare) (or equivalent) and then recompute
context["nuxt_module_name_bare"] = bare, context["nuxt_module_config_key"] =
parts[0] + "".join(p.capitalize() for p in parts[1:]), and
context["nuxt_module_name_pascal"] = "".join(p.capitalize() for p in parts) so
both hyphen- and underscore-delimited names produce correct PascalCase and
camelCase values (adjust imports to use the regex split where needed).
In `@template/.github/workflows/get_npm_version.py`:
- Around line 7-9: The extract_version function currently accesses
json.load(f)["version"] which raises a raw KeyError if the package.json lacks a
"version" key; change it to parse the JSON into a variable (e.g., data =
json.load(f)), check if "version" in data, and if missing raise a clearer
exception (ValueError or KeyError) with a descriptive message that includes the
path argument so CI logs show which package.json is invalid; keep the rest of
extract_version signature and behavior the same.
In `@template/.github/workflows/publish_to_staging.yaml.jinja`:
- Around line 78-81: The publish step sources ".
.devcontainer/code-artifact-auth.sh" then runs "pnpm publish --tag rc
--no-git-checks", but the helper template (code-artifact-auth.sh.jinja)
currently only emits real auth when "python_package_registry" == "AWS
CodeArtifact", so for npm-only CodeArtifact setups the sourced script is a no-op
and publish lacks registry auth; update the auth helper to also check
"npm_package_registry" (or add a new ".devcontainer/code-artifact-auth-npm.sh"
and source that here) so that when "npm_package_registry" indicates CodeArtifact
the script emits proper npm login/token export before running the "pnpm publish"
command.
In `@template/.github/workflows/publish.yaml.jinja`:
- Around line 134-137: The publish step sources
.devcontainer/code-artifact-auth.sh which is only generated when
python_package_registry == "AWS CodeArtifact", so in repos that set
npm_package_registry to CodeArtifact but not the Python setting this step is
unauthenticated; update the template so generation of
.devcontainer/code-artifact-auth.sh (or an npm-specific helper) is gated by
npm_package_registry instead of (or in addition to) python_package_registry, or
create a separate npm-only auth helper and source that from the "Publish to
CodeArtifact Staging" job (and the other publish jobs at the same locations) so
the pnpm publish --tag rc --no-git-checks step always has proper CodeArtifact
credentials when npm_package_registry is CodeArtifact.
- Around line 206-209: The GitHub Actions step named "Install from CodeArtifact
Staging" uses Bash-specific dot-sourcing (. .devcontainer/code-artifact-auth.sh)
but doesn't set the shell, so it can fail on Windows (pwsh); update that step to
include "shell: bash" directly under the step name and do the same for the
"Install from CodeArtifact Primary" step (the analogous run block at lines
~349–352) so both run blocks execute under Bash on Windows runners.
- Around line 284-285: The current publish step uses "pnpm publish
--no-git-checks" (step named "Publish to npm (promotes to latest)") which will
fail because the RC version is already published; replace that run command to
instead call npm dist-tag add on the already-published rc version and assign it
the "latest" tag (e.g. use the package name and version discovered from
package.json or from the staging job output and run npm dist-tag add
<package>@<version> latest), ensuring npm auth is available and reusing the rc
dist-tag version produced by the staging job rather than attempting to
republish.
In `@template/.npmrc`:
- Line 1: Remove the legacy-peer-deps=true setting from the template .npmrc so
generated projects do not inherit the legacy peer-deps behavior; edit the
template file to delete the legacy-peer-deps=true line and, if you need to allow
legacy peer installs in specific environments, apply legacy-peer-deps only in
targeted CI job steps instead of the template .npmrc.
In `@template/CHANGELOG.md`:
- Line 31: Replace the vague date placeholder in the changelog header "##
[0.0.1] - <insert date here>" with a clear format hint (e.g. "## [0.0.1] -
<YYYY-MM-DD>") so contributors know to use the Keep a Changelog/ISO format;
update that header text to use the explicit "<YYYY-MM-DD>" placeholder or
equivalent wording to indicate the expected format.
In `@template/eslint.config.mjs`:
- Around line 4-10: Remove non-essential inline comments in the template ESLint
config: delete the "Rules for module authors" and "Rules for formatting"
comments that appear inside the features block and trim any other authoring-only
notes; retain only comments that explain non-obvious behavior (e.g., the
top-line `npx `@eslint/config-inspector`` hint can stay if you deem it essential).
Update the createConfigForNuxt({ features: { tooling: true, stylistic: true }})
block to have no noisy comments so the template is minimal and only documents
non-obvious semantics.
In `@template/package.json.jinja`:
- Around line 25-27: The package.json uses the "workspaces" field with the
"playground" workspace but pnpm doesn't detect workspaces from package.json; add
a root pnpm-workspace.yaml that declares the workspace so pnpm installs the
playground package during CI/local installs (i.e., create pnpm-workspace.yaml
containing a packages section listing 'playground' to mirror the "workspaces":
["playground"] entry).
In `@template/playground/nuxt.config.ts.jinja`:
- Line 4: Replace the compatibilityDate value currently set to "latest" with a
concrete YYYY-MM-DD string to avoid behavior drift; update the compatibilityDate
token in the template (the compatibilityDate entry in nuxt.config.ts.jinja) to a
specific release date (e.g., the date you want to pin) and ensure any template
generation/tests use that exact string so builds remain deterministic.
In `@template/README.md.jinja`:
- Line 70: The template command has an extra space before the conditional which
renders a double space when is_open_source is true; move the space into the
conditional by changing the Jinja snippet that currently reads "pnpm publish
--tag rc {% if is_open_source %} --access public{% endif %}" so the leading
space is inside the if (use "{% if is_open_source %} --access public{% endif %}"
-> "{% if is_open_source %} --access public{% endif %}" with the space
immediately before --access public) or equivalently place the space immediately
before the --access token inside the conditional that uses the is_open_source
variable so no double space appears when rendered.
In `@template/src/runtime/plugin.ts.jinja`:
- Line 4: Remove the unconditional runtime console.log in
template/src/runtime/plugin.ts.jinja; either delete the line or wrap it behind a
development-only check (e.g., guard with process.env.NODE_ENV !== 'production'
or a specific env var like DEBUG_PLUGIN_INJECTION) so "Plugin injected by {{
npm_package_name }}!" only prints in dev, and update any related initialization
comments if present.
In `@template/test/basic.test.ts`:
- Line 12: The assertion uses a hardcoded HTML string ("<div>basic</div>") which
is a magic literal; replace it with a named constant or fixture (e.g.,
expectedContent or generatedRandomContent) and use that in the
expect(html).toContain(...) call so the test intent is explicit and less
brittle—locate the assertion that references html and update it to compare
against the new variable (or a randomized fixture) instead of the raw literal.
In `@tests/copier_data/data3.yaml`:
- Around line 10-13: The YAML file tests/copier_data/data3.yaml contains too
many consecutive blank lines at lines 10-13; reduce the blank lines to at most
two consecutive blank lines (i.e., remove 2 of the blank lines) so the file
passes yamllint's "max blank lines" rule and maintains the existing content
structure.
In `@tests/unit/test_jinja.py`:
- Around line 7-12: The test currently calls Path("template").rglob("*.jinja")
twice (once in idfn() and once in the `@pytest.mark.parametrize`), causing
non-deterministic/unstable IDs; fix by computing a single sorted list of
template Paths once (e.g., templates =
sorted(Path("template").rglob("*.jinja"))) and reuse that list for both the
param iterable and the ids (convert to strings for ids), updating idfn or
removing it and referencing that templates list in
test_jinja_templates_are_valid’s parametrize so IDs and parameters remain
aligned and stable; keep references to idfn, test_jinja_templates_are_valid, and
Path("template").rglob("*.jinja") when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3fa5e97-fbb0-46be-b87e-8ef62e9dcca3
⛔ Files ignored due to path filters (1)
template/{%if install_claude_cli %}.claude{% endif %}/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (55)
copier.ymlextensions/context.pytemplate/.devcontainer/devcontainer.json.jinjatemplate/.github/workflows/ci.yaml.jinjatemplate/.github/workflows/get_npm_version.pytemplate/.github/workflows/publish.yaml.jinjatemplate/.github/workflows/publish_to_staging.yaml.jinjatemplate/.npmrctemplate/CHANGELOG.mdtemplate/README.md.jinjatemplate/eslint.config.mjstemplate/package.json.jinjatemplate/playground/app.vuetemplate/playground/nuxt.config.ts.jinjatemplate/playground/package.json.jinjatemplate/playground/server/tsconfig.jsontemplate/playground/tsconfig.jsontemplate/src/module.ts.jinjatemplate/src/runtime/plugin.ts.jinjatemplate/src/runtime/server/tsconfig.jsontemplate/test/basic.test.tstemplate/test/fixtures/basic/app.vuetemplate/test/fixtures/basic/nuxt.config.ts.jinjatemplate/test/fixtures/basic/package.jsontemplate/tsconfig.jsontemplate/{% if install_claude_cli %}AGENTS.md{% endif %}template/{% if install_claude_cli %}CLAUDE.md{% endif %}template/{% if is_open_source %}CONTRIBUTING.md{% endif %}template/{%if install_claude_cli %}.claude{% endif %}/commands/add-command.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/commit.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/create-adr.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/create-issues.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/gap.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/green.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/issue.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/polish.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/red.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/refactor.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/research.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/simplify.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/spike.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/summarize.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/tdd-review.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/commands/tdd.mdtemplate/{%if install_claude_cli %}.claude{% endif %}/helpers/merge-claude-settings.shtemplate/{%if install_claude_cli %}.claude{% endif %}/package.jsontemplate/{%if install_claude_cli %}.claude{% endif %}/settings/basics.jsonctemplate/{%if install_claude_cli %}.claude{% endif %}/settings/permissions/bash.jsonctemplate/{%if install_claude_cli %}.claude{% endif %}/settings/permissions/read.jsonctemplate/{%if install_claude_cli %}.claude{% endif %}/settings/permissions/write.jsonctests/copier_data/data1.yamltests/copier_data/data2.yamltests/copier_data/data3.yamltests/unit/__init__.pytests/unit/test_jinja.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@template/.devcontainer/devcontainer.json.jinja`:
- Around line 9-35: The template contains a redundant Jinja token sequence "{%
raw %}{% endraw %}" between the block closing "{% endif %}" for node feature and
the conditional "{% if install_claude_cli %}" inside the features object; remove
the extraneous "{% raw %}{% endraw %}" so the conditional comma and the "{% if
install_claude_cli %}" remain adjacent (preserving the existing conditional
comma logic) in template/.devcontainer/devcontainer.json.jinja.
- Around line 54-61: Remove the redundant `{% raw %}{% endraw %}` tokens between
the VueJS and JavaScript conditional blocks in the Jinja template; locate the
conditional blocks that use the variables is_child_of_copier_base_template,
template_uses_vuejs, and template_uses_javascript in
template/.devcontainer/devcontainer.json.jinja and delete the unnecessary `{%
raw %}{% endraw %}` sequence so the surrounding Jinja conditionals flow directly
without the extra raw markers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8f4f2b0-ccdf-4c26-a273-b3404e16a11f
📒 Files selected for processing (3)
template/.devcontainer/devcontainer.json.jinjatemplate/.pre-commit-config.yamltemplate/ty.toml
💤 Files with no reviewable changes (2)
- template/ty.toml
- template/.pre-commit-config.yaml
ejfine
reviewed
May 5, 2026
| matrix: | ||
| os: | ||
| - "{{ gha_linux_runner }}" | ||
| {% if use_windows_in_ci %} - {{ gha_windows_runner }} |
Contributor
There was a problem hiding this comment.
NIT: same question as before, if we'd ever use windows for a Vue/TS library?
Contributor
Author
There was a problem hiding this comment.
sure its probably possible that we could but ill drop it and if we ever need it we can add it back.
ejfine
reviewed
May 5, 2026
| {% else %} | ||
| - name: Publish to npm (staging rc tag) | ||
| env: | ||
| NODE_AUTH_TOKEN: {% raw %}${{ secrets.NPM_TOKEN }}{% endraw %} |
Contributor
There was a problem hiding this comment.
just curious: I thought NPM got rid of token publishing and we had to use environments now
ejfine
reviewed
May 5, 2026
| shell: bash | ||
| run: | | ||
| . .devcontainer/code-artifact-auth.sh | ||
| npm install --prefix /tmp/staging-test "{{ npm_package_name }}@{% raw %}${{ needs.get-values.outputs.package-version }}{% endraw %}" --no-save |
Contributor
There was a problem hiding this comment.
NIT: I think there's an ENVVAR github provides for their temporary directory...not sure if they'd ever change it from /tmp at some point on their runners
ejfine
reviewed
May 5, 2026
Contributor
There was a problem hiding this comment.
NIT: I hadn't gotten around to pulling this out of the python template...but I think now that we have the flag in the main publish workflow that we don't even need this...?
ejfine
reviewed
May 5, 2026
| export default defineNuxtConfig({ | ||
| modules: ["{{ npm_package_name }}"], | ||
| devtools: { enabled: true }, | ||
| compatibilityDate: "latest", |
Contributor
There was a problem hiding this comment.
NIT: I think we have some compatibility date defined in the copier-nuxt-intranet-app repo
ejfine
reviewed
May 5, 2026
| @@ -0,0 +1,17 @@ | |||
| // @vitest-environment node | |||
Contributor
There was a problem hiding this comment.
NIT: do we need this? I thought node was the default
ejfine
reviewed
May 5, 2026
Contributor
There was a problem hiding this comment.
I can't tell if making the change like this prevents this from receiving changes to the file that come from copier-base in the future. I know when I renamed .pre-commit-config.yaml to .pre-commit-config.yaml.jinja in copier-nuxt-intranet-app that it stopped receiving updates from copier-base
Contributor
Author
There was a problem hiding this comment.
make this not jinja for now for all ai stuff. Make sure matches upstream. Will revisit that later
ejfine
reviewed
May 5, 2026
Contributor
There was a problem hiding this comment.
Future thought: maybe at some point we pull the eslint config up into copier base, so it can fan out into both this template and the copier-nuxt-intranet-app
ejfine
reviewed
May 5, 2026
| - Prefer dedicated shell tools over `python3`/`python` for simple one-off tasks: use `jq` for JSON parsing, standard shell builtins for string manipulation, etc. Only reach for `python3` when no simpler tool covers the need. | ||
| - Check .devcontainer/devcontainer.json for tooling versions (Python, Node, etc.) when reasoning about version-specific stdlib or tooling behavior. | ||
| - For frontend tests, run commands via `pnpm` scripts from `frontend/package.json` — never invoke tools directly (not pnpm exec <tool>, npx <tool>, etc.). ✅ pnpm test-unit ❌ pnpm vitest ... or npx vitest ... | ||
| - For frontend tests, run commands via `pnpm` scripts from `frontend/package.json` — never invoke tools directly (not pnpm exec <tool>, npx <tool>, etc.). ✅ pnpm test-unit ❌ 1111111111111111111 vitest ... or npx vitest ... |
Contributor
There was a problem hiding this comment.
did a cat jump on the keyboard?
|
|
||
|
|
||
| @pytest.mark.parametrize("jinja_template", Path("template").rglob("*.jinja"), ids=idfn()) | ||
| def test_jinja_templates_are_valid(jinja_template: Path): |
Contributor
There was a problem hiding this comment.
curious, I thought we had a jinja lint precommit hook...? or was that in a past life?
Contributor
Author
There was a problem hiding this comment.
yah just trying to get faster feedback. Real time we talked and want to go down the pre-commit hook
Contributor
Author
There was a problem hiding this comment.
LabAutomationAndScreening/copier-base-template#156 created this in base and will do a followup to clean this up and have it added to all copier templates.
ejfine
approved these changes
May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this change necessary?
We want a copier template that can build out reusable nuxt modules
How does this change address the issue?
Creates the framework to allow for scaffolding out all things necessary to make that possible.
What side effects does this change have?
N/A
How is this change tested?
In CI and creating a new nuxt module from it and verifying it works as expected.
Summary by CodeRabbit
New Features
Configuration
Documentation
Tests
Chores