Skip to content

[codex] Add create-frappe-ui initializer - #826

Open
netchampfaris wants to merge 2 commits into
mainfrom
codex/create-frappe-ui-cli
Open

[codex] Add create-frappe-ui initializer#826
netchampfaris wants to merge 2 commits into
mainfrom
codex/create-frappe-ui-cli

Conversation

@netchampfaris

@netchampfaris netchampfaris commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Adds a separate create-frappe-ui package under packages/create-frappe-ui so npm create frappe-ui can resolve to a dedicated initializer.
  • Ships a TypeScript-only Vue starter with shared base files and separate plain/Frappe overlays.
  • Adds CLI prompts and flags for Frappe backend setup:
    • --frappe / --no-frappe
    • --route <path> for the frontend base route
    • validation that Frappe-backed projects are created inside apps/<appname>/<folder>
  • Updates getting-started docs and the README starter link to use the new initializer.

Notes

Plain projects still use frappe-ui/vite with Frappe backend features disabled so the Frappe UI icon resolver is available without enabling proxy/build/Jinja behavior.

Validation

  • node bin/create-frappe-ui.js /tmp/frappe-ui-plain-ts --no-frappe
  • cd /tmp/frappe-ui-plain-ts && npm install && npm run build
  • node bin/create-frappe-ui.js /tmp/frappe-bench-test/apps/todo/frontend --frappe --route desk/frontend
  • cd /tmp/frappe-bench-test/apps/todo/frontend && npm install && npm run build
  • Prompt flow with Frappe backend enabled and default /frontend route
  • Invalid Frappe target path exits with an error
  • npm pack --dry-run --json
  • yarn docs:build using the original checkout's node_modules; it exits 0 with existing VitePress/router warnings during rendering

Docs preview: https://ui.frappe.io/pr-preview/pr-826/

Coverage: 68.80% (-0.01% vs main)

@netchampfaris
netchampfaris marked this pull request as ready for review July 6, 2026 05:23
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

This is close, but these issues should be fixed before merging.

  • Route values are escaped for generated TypeScript, but not validated before they reach build output paths.
  • Forced scaffolding can still overwrite user-owned files without a file-level guard.

packages/create-frappe-ui/bin/create-frappe-ui.js

Security Review

Accepted route values can still contain traversal segments that flow into build-time file paths.

Reviews (2): Last reviewed commit: "fix(create): address initializer review ..." | Re-trigger Greptile

Comment thread packages/create-frappe-ui/bin/create-frappe-ui.js
Comment on lines +203 to +208
const contents = fs
.readFileSync(sourcePath, 'utf8')
.replaceAll('__PROJECT_NAME__', context.projectName)
.replaceAll('__BASE_ROUTE__', context.baseRoute)
.replaceAll('__ROUTER_BASE__', context.routerBase)
fs.writeFileSync(targetPath, contents)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Forced Copy Overwrites Files

With --force, every matching template file is written unconditionally. Re-running the initializer in an existing app can replace edited files like .gitignore, package.json, or src/main.ts without any per-file warning, causing local project changes to be lost.

Context Used: Guidelines for reviewing Frappe Framework applicat... (source)

const context = {
baseRoute: options.route,
baseRouteCode: JSON.stringify(options.route),
projectName: toPackageName(path.basename(targetDir)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 security Validate Route Paths --route is escaped for TypeScript, but values like /../../foo are still accepted and written to frontendRoute. During a Frappe build, that route is used to derive the generated HTML output path, so traversal segments can write outside the intended www route file; reject .., backslashes, and filesystem separators before generating the config.

.replaceAll('__BASE_ROUTE_CODE__', context.baseRouteCode)
.replaceAll('__ROUTER_BASE__', context.routerBase)
.replaceAll('__ROUTER_BASE_CODE__', context.routerBaseCode)
fs.writeFileSync(targetPath, contents)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Guard Existing Files --force still writes every matching template path unconditionally. Re-running the initializer in an existing app can replace user-edited files such as package.json, src/router.ts, or vite.config.ts; add per-file confirmation, backup, or skip handling before writing an existing path.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant