Skip to content

Commit 88c81cf

Browse files
afuchereca-agent
andauthored
Doc website (#109)
* Issue 93 setup astro (#104) * feat: Set up Astro + Starlight project structure (Issue #93) Implements the initial website directory with: - Astro + Starlight configuration - TypeScript setup (tsconfig.json) - Component structure (ChartDemo, SinceBadge) - Custom landing page (src/pages/index.astro) - Documentation pages structure (src/content/docs/) * Getting Started: introduction, installation, quickstart * Guides: radix, transit, animation, custom settings, etc. * Framework integrations: React, Vue, Angular * API Reference: Chart, Radix, Transit, Settings, Types, etc. * Project: changelog, contributing - Placeholder logo and styles - .gitignore and environment setup Acceptance criteria met: ✓ website/ directory created with package.json ✓ astro.config.mjs configured with Starlight ✓ TypeScript setup ✓ src/ directory structure with pages, components, styles, content/docs ✓ public/ directory with placeholder logo ✓ ChartDemo and SinceBadge components ready Next: npm install and verify build (will be done in Phase 1) * fix: Resolve build errors for Astro 6 + Starlight 0.38 compatibility - Fix tsconfig.json: correct Astro tsconfig preset path (configs/ -> tsconfigs/) - Upgrade to astro@^6.0.0 + @astrojs/starlight@^0.38.0 (Zod v4 compatible) - Add .npmrc with legacy-peer-deps=true for peer dependency resolution - Fix src/content.config.ts location and add docsLoader() (Astro 6 requirement) - Fix astro.config.mjs sidebar slugs (remove 'docs/' prefix) - Fix social config syntax (array instead of object, Starlight v0.33+ change) - Rewrite landing page to use StarlightPage component correctly - Fix internal links to use correct route paths Build result: 24 pages built, search index generated, sitemap created. * docs: Add retrospective findings to AGENTS.md - Add Environment section (nvm use 24, website sub-project) - Add Adding New Dependencies section with verification workflow * fix: Exclude website/ from root TypeScript and webpack build The website/ sub-project has its own node_modules with Astro/Starlight packages. Without exclusions, ts-loader was crawling into website/node_modules/ and failing with 24 TypeScript errors. - tsconfig.json: add 'website' to exclude list - webpack.config.js: add /website/ to ts-loader exclude regex * docs: Add sub-project isolation hard rule to AGENTS.md * [Phase 1] Implement ChartDemo component and demo data (#105) * feat(website): implement ChartDemo component and demo data (#94) - Add `src/data/demoData.ts` with defaultRadixData (15 planets + 12 cusps) and defaultTransitData for transit/animate examples - Rewrite ChartDemo.astro with updated props: id, height (default 500), mode ('radix'|'transit'|'animate'), showCode (default true) - Animate mode renders a 'Start Animation' button wired to transit.animate() - Inline <script> loads /astrochart.js on demand and initialises chart based on mode, using demoData passed via define:vars - Collapsible <details> code snippet auto-generated per mode - <noscript> fallback included - Add website/public/astrochart.js (UMD bundle served to browser) - Add src/content/docs/test-demo.md testing all modes and props - Website builds cleanly (25 pages); root lib build + 86 tests all pass 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * fix(website): fix ChartDemo not rendering on test-demo page Three bugs fixed: 1. Rename test-demo.md → test-demo.mdx Astro/Starlight only processes component imports (MDX syntax) in .mdx files. The .md file was printing the import statement as plain text and not rendering any <ChartDemo> tags. 2. Fix planet key names in demoData.ts The library recognises NNode/SNode/Fortune; the data used the non-existent keys NorthNode, SouthNode, Vertex which caused the validate() call inside Radix to silently skip those planets and could throw on malformed data shapes. 3. Fix multi-instance script loading race condition With 5 ChartDemo instances on one page, all inline scripts ran simultaneously and each tried to inject /astrochart.js. Replace with a shared __astrochartQueue pattern: the first instance creates the script tag and flushes the queue on load; subsequent instances detect the in-flight tag and push their initChart() to the queue instead of creating duplicate script tags. 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * docs: apply learnings from Issue #94 debugging AGENTS.md: - Add website/Astro rules: .mdx required for component imports; queue pattern for multi-instance is:inline script loading - Add AstroChart data shape section: real AstroData type, valid planet keys, cusps constraint, retrograde flag format website docs (wrong data shape fixed): - api/types.md: rewrite with real AstroData/Points types, table of valid planet keys, full working example - guides/radix-chart.md: replace invented Planet[]/Cusp[] API with real Record<string,number[]>/number[] shape throughout - api/radix.md: correct code example, document data constraints library-issues/ (new directory, two files): - improve-validate-unknown-planet-keys.md: unknown planet keys silently render as fallback circles with no warning — improvement candidate - bug-settings-mutation-across-instances.md: Chart constructor mutates the shared default_settings singleton via Object.assign, causing cross-instance settings bleed — confirmed bug 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * docs: remove pointless prop-test sections from test-demo 'Radix with Custom Height' and 'Transit without Code Snippet' existed to exercise ChartDemo component props — not to communicate anything useful to a documentation reader. Removed. * fix: exclude project/__tests__ from TypeScript compilation Without this exclusion tsc emits .d.ts files for the test helpers into dist/project/__tests__/ on every npm run build. The issue-93 branch had this fix but it was not in doc-website, so it reappeared on the new branch. --------- Co-authored-by: eca <git@eca.dev> * [Phase 1] Create custom landing page (#106) * [Phase 1] Create custom landing page Implement comprehensive landing page for AstroChart with all required sections: Hero Section: - Tagline: "A free and open-source JavaScript library for generating SVG astrology charts" - npm install snippet - "Get Started" CTA → /installation - "View on GitHub" link Version Badge: - Dynamically reads version from package.json at build time - Displays as "v3.0.2" - Links to /changelog Live Demo Section: - Side-by-side radix and transit chart demos using ChartDemo component - Collapsible code example showing usage - Interactive charts powered by pure SVG Feature Cards: - Pure SVG: crisp, scalable vector graphics - Zero Dependencies: pure vanilla JavaScript - Fully Customizable: control all visual settings - TypeScript-first: full type definitions included Sponsorship Section: - Ko-fi button with coffee emoji - GitHub Sponsors button with heart emoji - Gradient background styling Mobile Responsive: - Works on 320px–1920px viewports - Grid layouts adjust to single column on mobile - Sponsor buttons stack vertically on narrow screens Build verified: 25 pages built successfully with no errors Closes #95 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * fix sponsor link --------- Co-authored-by: eca <git@eca.dev> * [Phase 1] Write core documentation pages (#107) * [Phase 1] Write core documentation pages ✅ Issue #97 Complete ## Pages Created/Updated 1. **introduction.mdx** — Welcome page with feature overview - Explains what AstroChart is and is NOT - Feature table highlighting key capabilities - Inline <ChartDemo> showing a basic radix chart - Links to GitHub, npm, and next steps 2. **installation.md** — Installation guide - npm, yarn, pnpm package managers - CDN unpkg bundle with UMD global example - ESM import for bundlers - TypeScript type definitions - Compatibility table for different environments 3. **quickstart.mdx** — Quick Start guide - 4-step walkthrough: Install → Container → Data → Render - Data format explanation (Record<string, number[]> for planets, array of 12 for cusps) - Complete HTML + JS example (copy-pasteable) - Live <ChartDemo> showing rendered output - Troubleshooting section 4. **guides/radix-chart.mdx** — Radix Chart guide - Full explanation of radix charts - Data format details with valid planet names - House cusps requirement (exactly 12 values) - Complete example with all 15 planets/points - Retrograde planet marking - Aspects with customizable orbs - Live <ChartDemo> example 5. **guides/transit-chart.mdx** — Transit Chart guide - How transit charts overlay on radix - Complete examples with both radix and transit data - Retrograde transit planets - Transit-to-natal aspects - Live <ChartDemo> showing dual-ring chart - API reference for transit methods 6. **guides/animation.mdx** — Animation guide (greatly expanded) - Detailed animation method signature and parameters - Complete interactive example with date picker - Duration best practices (300ms–5s recommendations) - Callback patterns and completion handling - Chaining animations with async/await - Performance considerations - Common patterns (date input, continuous loop) - Live <ChartDemo mode="animate"> with interactive button - Troubleshooting guide ## Key Changes - Converted 5 files from .md to .mdx to support component imports - Fixed all data format examples to use correct AstroData structure: - `planets: Record<string, number[]>` (was array of objects) - `cusps: number[]` of exactly 12 values - Added `<ChartDemo>` components to intro, quickstart, and all 3 guides - Enhanced animation guide with complete API documentation - Verified all code examples match actual library API - All pages include proper links to related docs ## Build Verification ✅ Build successful: 25 pages generated ✅ All 6 core docs pages render without errors ✅ Pagefind search index includes all new pages ✅ No TypeScript diagnostics ✅ No broken internal links ✅ No console errors 🧑 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * chore: rebuild dist bundle Regenerate astrochart.js UMD bundle via `npm run build`. No source changes — output is deterministic from current TypeScript sources. 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * docs: fix planet velocity description — not a retrograde flag The second element of a planet array is the astrological velocity of the point, not a boolean/flag. A negative velocity means the planet is retrograde; the sign of the value carries the meaning, not an arbitrary sentinel like -1. Updated in all affected files: - quickstart.mdx - guides/radix-chart.mdx - guides/transit-chart.mdx - api/types.md - api/radix.md Example values in code snippets updated to use realistic velocities (e.g. -1.5, -0.3) instead of the misleading -1 flag convention. 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> --------- Co-authored-by: eca <git@eca.dev> * [Phase 1] Configure and test CI/CD pipeline (#96) (#108) * ci: add docs-deploy workflow and deployment guide (#96) - Add .github/workflows/docs-deploy.yml: - Triggers on push to main (paths: website/** and project/src/**) - Supports manual trigger via workflow_dispatch - Builds library → copies bundle → builds Astro site → deploys to AstroDraw/astrodraw.github.io via peaceiris/actions-gh-pages - Uses DOCS_DEPLOY_KEY secret for SSH deploy key authentication - Add docs/docs-deploy.md: - Explains what each workflow step does - One-time SSH key pair setup instructions - How to manually trigger via GitHub Actions UI - Debugging guide for common failure scenarios Closes #96 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * ci: switch to official GitHub Pages action, deploy from AstroChart repo (#96) Replace peaceiris/actions-gh-pages (third-party, slow to maintain, requires SSH deploy key setup) with the official GitHub Actions stack: - actions/upload-pages-artifact@v3 - actions/deploy-pages@v4 Changes: - Split single job into build + deploy jobs - Add top-level permissions (pages: write, id-token: write, contents: read) - Add concurrency group to prevent overlapping deploys - Remove all external repo / SSH deploy key config — no secrets needed - Site now deploys directly to AstroChart repo Pages Update website/astro.config.mjs: - site: https://astrodraw.github.io/AstroChart - base: /AstroChart (required for project repo sub-path deployment) Update docs/docs-deploy.md to reflect new one-time setup (Pages source → GitHub Actions in repo settings) and remove SSH key instructions. 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev> * ci: temporarily enable workflow trigger on issue-96-ci-cd for testing * ci: revert temporary test trigger on issue-96-ci-cd --------- Co-authored-by: eca <git@eca.dev> --------- Co-authored-by: eca <git@eca.dev>
1 parent 3959fd4 commit 88c81cf

46 files changed

Lines changed: 9851 additions & 3 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/docs-deploy.yml

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
name: Deploy Docs
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
paths:
8+
- 'website/**'
9+
- 'project/src/**'
10+
workflow_dispatch:
11+
12+
permissions:
13+
contents: read
14+
pages: write
15+
id-token: write
16+
17+
concurrency:
18+
group: pages
19+
cancel-in-progress: false
20+
21+
jobs:
22+
build:
23+
runs-on: ubuntu-22.04
24+
steps:
25+
- name: Checkout
26+
uses: actions/checkout@v4
27+
28+
- name: Set up Node.js
29+
uses: actions/setup-node@v4
30+
with:
31+
node-version: 20
32+
33+
- name: Install root dependencies
34+
run: npm ci
35+
36+
- name: Build library
37+
run: npm run build
38+
39+
- name: Copy bundle to website
40+
run: cp dist/astrochart.js website/public/astrochart.js
41+
42+
- name: Install website dependencies
43+
run: npm ci
44+
working-directory: website
45+
46+
- name: Build Astro site
47+
run: npm run build
48+
working-directory: website
49+
50+
- name: Upload Pages artifact
51+
uses: actions/upload-pages-artifact@v3
52+
with:
53+
path: website/dist
54+
55+
deploy:
56+
needs: build
57+
runs-on: ubuntu-22.04
58+
environment:
59+
name: github-pages
60+
url: ${{ steps.deployment.outputs.page_url }}
61+
steps:
62+
- name: Deploy to GitHub Pages
63+
id: deployment
64+
uses: actions/deploy-pages@v4

AGENTS.md

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# AGENTS.md
2+
3+
## Environment
4+
- Node: use `nvm use 24` if node commands fail
5+
- Website sub-project lives in `website/` with its own `package.json`; run npm commands from there
6+
7+
## Build / Lint / Test
8+
- Install: `npm ci`
9+
- Build: `npm run build` (webpack UMD bundle → `dist/astrochart.js`)
10+
- Lint: `npm run lint` (ESLint, TypeScript source files only)
11+
- Test all: `npm test` (Jest + ts-jest, jsdom environment)
12+
- Test single file: `npx jest project/src/utils.test.ts`
13+
- Test with coverage: `npm run test:coverage`
14+
15+
## Code Style
16+
- **Formatting:** 2-space indent, single quotes, no semicolons, unix line endings, no trailing commas, no `var`
17+
- **Functions:** class methods have a space before parens (`radix (data: AstroData) {`); standalone functions use `export const fn = (...) => { ... }`
18+
- **Naming:** Classes/interfaces PascalCase, methods/variables camelCase, settings keys UPPER_SNAKE_CASE, files lowercase single-word
19+
- **Imports:** default imports for classes, named imports for functions, `import type` for type-only; relative `./` paths, no extensions, no aliases
20+
- **Types:** interfaces/types live in the file where primarily used — no separate types file
21+
- **Tests:** co-located (`foo.test.ts` next to `foo.ts`), use `describe`/`test` (not `it`), prefer `toStrictEqual`, never commit `.only`
22+
- **Errors:** throw plain `Error('descriptive message')`, no custom error classes; null checks use loose equality (`== null`)
23+
- **Docs:** JSDoc on public methods/classes with `@param`, `@return` tags
24+
- **⚠️ Breaking changes:** this is a production library with many consumers — never change public API (exported types, method names, function signatures)
25+
26+
## Adding New Dependencies
27+
- Never write import paths or config shapes from memory for fast-moving packages (Astro, Starlight, etc.)
28+
- After `npm install`, verify real exports: `cat node_modules/<pkg>/package.json | python3 -c "import json,sys; d=json.load(sys.stdin); print(list(d.get('exports',{}).keys()))"`
29+
- Run `npm run build` (or `dev`) after creating the first file — don't build 30 files then discover the config is wrong
30+
- Use `legacy-peer-deps=true` in `.npmrc` when a package's peer range lags behind the latest patch
31+
32+
## Sub-projects isolation (⚠️ hard rule)
33+
- `website/` is a completely separate project — it must **never** affect the library build or tests
34+
- Any new sub-project directory **must** be added to the root `tsconfig.json` `exclude` list AND to the `exclude` regex in `webpack.config.js` before committing
35+
- After adding a sub-project, always run `npm run build` and `npm test` from the **root** to verify isolation
36+
37+
## Website / Astro content rules
38+
- **MDX required for component imports:** Starlight content files that use `import` and JSX component tags **must** have a `.mdx` extension. A `.md` file will print the import statement as plain text and silently ignore all component tags.
39+
- **Multi-instance inline script loading:** When an Astro `is:inline` script dynamically loads an external JS bundle, multiple component instances on the same page will all run simultaneously. Use a shared queue pattern to avoid race conditions:
40+
```javascript
41+
if (window.astrochart) {
42+
initChart()
43+
} else if (document.querySelector('script[src="/astrochart.js"]')) {
44+
window.__astrochartQueue = window.__astrochartQueue || []
45+
window.__astrochartQueue.push(initChart)
46+
} else {
47+
window.__astrochartQueue = [initChart]
48+
const s = document.createElement('script')
49+
s.src = '/astrochart.js'
50+
s.onload = () => { (window.__astrochartQueue || []).forEach(fn => fn()); window.__astrochartQueue = [] }
51+
document.head.appendChild(s)
52+
}
53+
```
54+
55+
## AstroChart library — data shape
56+
The real `AstroData` type (from `project/src/radix.ts`) is:
57+
```typescript
58+
interface AstroData {
59+
planets: Record<string, number[]> // key = symbol name, value = [degrees, retrograde?]
60+
cusps: number[] // exactly 12 degree values
61+
}
62+
```
63+
- **Valid planet keys** (anything else renders as a red fallback circle with no warning):
64+
`Sun`, `Moon`, `Mercury`, `Venus`, `Mars`, `Jupiter`, `Saturn`, `Uranus`, `Neptune`, `Pluto`, `Chiron`, `Lilith`, `NNode`, `SNode`, `Fortune`
65+
- **Cusps** must be an array of **exactly 12** numbers (degrees); fewer or more will throw via `validate()`
66+
- **Retrograde:** second element of a planet array — negative value = retrograde (e.g. `[245.5, -1]`)
67+
- Do **not** use the invented `Planet[]`/`Cusp[]` shape that appears in older placeholder docs — it does not match the library

dist/astrochart.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/docs-deploy.md

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# Docs Deployment Guide
2+
3+
This document explains how the AstroChart documentation site is built and deployed to GitHub Pages directly from this repository.
4+
5+
The live site is served at **https://astrodraw.github.io/AstroChart/**.
6+
7+
---
8+
9+
## What the Workflow Does
10+
11+
The workflow file is located at `.github/workflows/docs-deploy.yml`.
12+
13+
It runs on every push to `main` that touches either `website/**` or `project/src/**`, and can also be triggered manually.
14+
15+
The workflow has two jobs:
16+
17+
### Job 1: `build`
18+
19+
1. **Checkout** — checks out the `AstroChart` repository.
20+
2. **Set up Node.js 20** — installs Node using `actions/setup-node`.
21+
3. **Install root dependencies** — runs `npm ci` at the repo root.
22+
4. **Build library** — runs `npm run build` to produce `dist/astrochart.js` (webpack UMD bundle).
23+
5. **Copy bundle to website** — copies `dist/astrochart.js``website/public/astrochart.js` so the live demos can load it.
24+
6. **Install website dependencies** — runs `npm ci` inside `website/`.
25+
7. **Build Astro site** — runs `npm run build` inside `website/`, outputting the static site to `website/dist/`.
26+
8. **Upload Pages artifact** — uploads `website/dist/` as a GitHub Pages artifact using the official `actions/upload-pages-artifact` action.
27+
28+
### Job 2: `deploy`
29+
30+
1. **Deploy to GitHub Pages** — deploys the uploaded artifact using the official `actions/deploy-pages` action. Uses GitHub's OIDC token — no secrets or SSH keys needed.
31+
32+
---
33+
34+
## One-Time Setup (GitHub Pages source)
35+
36+
This only needs to be done once per repository.
37+
38+
1. Go to the repository **Settings → Pages**:
39+
`https://github.com/AstroDraw/AstroChart/settings/pages`
40+
2. Under **Source**, select **GitHub Actions** (not a branch).
41+
3. Click **Save**.
42+
43+
That's it. The workflow handles everything else automatically.
44+
45+
> **Note:** GitHub automatically creates a `github-pages` environment on the first successful deploy. You can see it under **Settings → Environments**.
46+
47+
---
48+
49+
## How to Manually Trigger the Workflow
50+
51+
You can trigger a deploy at any time without pushing code:
52+
53+
1. Go to the **Actions** tab in the AstroChart repository:
54+
`https://github.com/AstroDraw/AstroChart/actions/workflows/docs-deploy.yml`
55+
2. Click **Run workflow**
56+
3. Select the branch (typically `main`)
57+
4. Click **Run workflow**
58+
59+
The workflow will run and deploy the current state of `main` to GitHub Pages.
60+
61+
---
62+
63+
## How to Debug Deploy Failures
64+
65+
### Step 1 — Check the workflow logs
66+
67+
Go to `https://github.com/AstroDraw/AstroChart/actions` and click the failed run. Each step's output is expandable. Common failure points:
68+
69+
| Step | Likely cause |
70+
|---|---|
71+
| Build library | TypeScript compile error or missing dep — run `npm run build` locally |
72+
| Build Astro site | MDX/Astro error — run `cd website && npm run build` locally |
73+
| Upload Pages artifact | `website/dist/` is empty or missing — check the Astro build step above it |
74+
| Deploy to GitHub Pages | Pages source not set to "GitHub Actions" — see one-time setup above |
75+
76+
### Step 2 — Permissions error on deploy
77+
78+
If the deploy job fails with a permissions error:
79+
80+
- Verify the repository **Settings → Pages → Source** is set to **GitHub Actions**, not a branch.
81+
- Verify the workflow has the correct top-level permissions (`pages: write`, `id-token: write`).
82+
- Check that the `deploy` job declares `environment: name: github-pages`.
83+
84+
### Step 3 — Verify the deployed site
85+
86+
After a successful run:
87+
88+
1. Visit `https://astrodraw.github.io/AstroChart/` — homepage should load.
89+
2. Open browser DevTools → Network tab — no 404s for `/AstroChart/astrochart.js`.
90+
3. Click through the sidebar — navigation should work.
91+
4. Open the browser console — no JS errors.
92+
93+
If the site shows stale content, GitHub Pages may have a CDN cache delay of up to 10 minutes after deployment.
94+
95+
---
96+
97+
## Astro base path configuration
98+
99+
The `website/astro.config.mjs` file is configured with:
100+
101+
```js
102+
site: 'https://astrodraw.github.io/AstroChart',
103+
base: '/AstroChart',
104+
```
105+
106+
The `base` option tells Astro to prefix all internal links and asset URLs with `/AstroChart`, which is required for a project repository deployed to a sub-path.
107+
108+
If the site is ever moved to the root URL (`https://astrodraw.github.io/`), remove the `base` option and update `site` accordingly.
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# Bug: `Chart` constructor mutates the shared `default_settings` singleton
2+
3+
**Type:** Bug
4+
**Affects:** `project/src/chart.ts``Chart` constructor
5+
**Severity:** Medium — causes cross-instance settings bleed when multiple charts are on the same page with different custom settings
6+
7+
---
8+
9+
## Current behaviour
10+
11+
`default_settings` is imported as a module-level singleton object. In the `Chart` constructor, custom settings are merged into it **in-place** via `Object.assign`:
12+
13+
```typescript
14+
// chart.ts — current constructor
15+
const chartSettings = default_settings // ← just a reference, not a copy
16+
if (settings != null) {
17+
Object.assign(chartSettings, settings) // ← mutates the shared singleton!
18+
...
19+
}
20+
```
21+
22+
Because `chartSettings` is a reference to the same object as `default_settings`, any settings passed to one `Chart` instance permanently modify the module-level default for all subsequent `Chart` instances in the same page/process.
23+
24+
## Reproduction
25+
26+
```javascript
27+
import { Chart } from '@astrodraw/astrochart'
28+
29+
// First chart — custom red background
30+
const chart1 = new Chart('chart1', 600, 600, { COLOR_BACKGROUND: '#ff0000' })
31+
32+
// Second chart — no custom settings passed, expects white background
33+
const chart2 = new Chart('chart2', 600, 600)
34+
// ❌ chart2 ALSO has a red background because default_settings was mutated
35+
```
36+
37+
## Expected behaviour
38+
39+
Each `Chart` instance should have its own isolated copy of the settings. The module-level `default_settings` must remain pristine.
40+
41+
## Suggested fix
42+
43+
Replace the reference assignment with a deep copy:
44+
45+
```typescript
46+
// chart.ts — fixed constructor
47+
constructor (elementId: string, width: number, height: number, settings?: Partial<Settings>) {
48+
// Create a fresh copy of defaults for this instance
49+
const chartSettings: Settings = { ...default_settings }
50+
51+
if (settings != null) {
52+
Object.assign(chartSettings, settings)
53+
if (!('COLORS_SIGNS' in settings)) {
54+
chartSettings.COLORS_SIGNS = [
55+
chartSettings.COLOR_ARIES, chartSettings.COLOR_TAURUS,
56+
// ... rest of sign colours
57+
]
58+
}
59+
}
60+
// ...
61+
}
62+
```
63+
64+
For nested objects (e.g. `ASPECTS`, `DIGNITIES_EXACT_EXALTATION_DEFAULT`) a shallow spread may not be enough — consider `structuredClone(default_settings)` if those nested objects are also mutated downstream.
65+
66+
## Notes
67+
68+
- The bug may be masked in most use cases where only one `Chart` instance is created per page, or where the same settings are reused
69+
- It is reproducible whenever two `Chart` instances with *different* custom settings are created in the same JavaScript context
70+
- Should be covered by a new test: create two Chart instances in sequence with conflicting settings and assert each uses its own values
71+
- The `COLORS_SIGNS` re-computation block inside the `if (settings != null)` branch also references `default_settings.COLOR_*` — after the fix it should reference `chartSettings.COLOR_*` instead (already correct behaviour, just noting for the reviewer)
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# Improvement: `validate()` does not check for unknown planet keys
2+
3+
**Type:** Improvement (not a breaking bug)
4+
**Affects:** `project/src/utils.ts``validate()`
5+
**Discovered during:** Issue #94 — writing demo data for the website
6+
7+
---
8+
9+
## Current behaviour
10+
11+
The `validate()` function checks that:
12+
- `data.planets` exists
13+
- each planet value is an `Array`
14+
- `data.cusps` is an Array of exactly 12 values
15+
16+
It does **not** check whether planet keys are in the set of recognised symbol names.
17+
18+
```typescript
19+
// validate() — current loop (utils.ts)
20+
for (const property in data.planets) {
21+
if (!Array.isArray(data.planets[property])) {
22+
status.messages.push(...)
23+
status.hasError = true
24+
}
25+
// ↑ only validates the value shape — the KEY is never checked
26+
}
27+
```
28+
29+
As a result, passing an unrecognised key (e.g. `NorthNode`, `Vertex`, `PartOfFortune`) **silently succeeds** validation and the library renders a generic red fallback circle at that position — with no warning to the developer.
30+
31+
## How it was discovered
32+
33+
When writing `demoData.ts` for the website, the following keys were used by mistake:
34+
35+
```typescript
36+
// ❌ These do NOT work — wrong key names
37+
NorthNode: [95.45, 0],
38+
SouthNode: [275.45, 0],
39+
Vertex: [325.67, 0]
40+
41+
// ✅ Correct names
42+
NNode: [95.45, 0],
43+
SNode: [275.45, 0],
44+
Fortune: [325.67, 0]
45+
```
46+
47+
The chart appeared to load but no symbols were drawn for those planets — no console error, no validation message.
48+
49+
## Expected behaviour
50+
51+
`validate()` should emit a **warning** (not a hard error, to avoid breaking existing charts) when it encounters a key that is not in the recognised symbol list:
52+
53+
```
54+
"Unknown planet key 'NorthNode'. Valid keys are: Sun, Moon, Mercury, Venus, Mars, Jupiter, Saturn, Uranus, Neptune, Pluto, Chiron, Lilith, NNode, SNode, Fortune."
55+
```
56+
57+
## Suggested fix
58+
59+
```typescript
60+
// In utils.ts, extend validate() with an optional warning step:
61+
const KNOWN_PLANET_KEYS = new Set([
62+
'Sun', 'Moon', 'Mercury', 'Venus', 'Mars', 'Jupiter',
63+
'Saturn', 'Uranus', 'Neptune', 'Pluto', 'Chiron',
64+
'Lilith', 'NNode', 'SNode', 'Fortune'
65+
])
66+
67+
for (const property in data.planets) {
68+
if (!Array.isArray(data.planets[property])) {
69+
status.messages.push(`The planets property '${property}' has to be Array.`)
70+
status.hasError = true
71+
} else if (!KNOWN_PLANET_KEYS.has(property)) {
72+
// Warning only — unknown keys are allowed (custom symbols),
73+
// but we surface the information to help developers catch typos
74+
console.warn(`[AstroChart] Unknown planet key '${property}'. It will render as a fallback symbol.`)
75+
}
76+
}
77+
```
78+
79+
## Notes
80+
81+
- This should be a **`console.warn`**, not an error — unknown keys with a `CUSTOM_SYMBOL_FN` setting are a valid use case
82+
- The `DEBUG` settings flag could gate the warning if preferred
83+
- Should be covered by a new unit test in `utils.test.ts`

0 commit comments

Comments
 (0)