Skip to content

Conversation

@charliecreates
Copy link

@charliecreates charliecreates bot commented Nov 3, 2025

Refreshes README and all docs under docs/ to match the v7 rewrite (css-tree parser, pure ESM) and to address discrepancies called out in #152.

Changes

  • Clarify parsing engine: css-tree is used for parsing; nodes extend PostCSS classes for a familiar API.
  • Update Requirements: Node >= 20.19.0; PostCSS >= 8.4.14 as a peer.
  • ESM-only: add explicit guidance (and convert many examples to import syntax).
  • Walker helpers: not auto-registered — document registerWalkers(Container) and update Root/Container/Walker docs and examples.
  • Remove/clarify unsupported parse() options in v7; mark ParseOptions as forward-compat only.
  • Fix Examples that suggested SCSS/LESS variable prefixes; note that Word.isVariable only detects --* in v7.
  • Stringifier docs: state that spacing is normalized (no attempt to preserve original whitespace).
  • README: replace legacy badges (CircleCI/Codecov) with GitHub Actions; update Benefits.
  • Minor correctness/consistency fixes across node docs (Comment, Punctuation, Parentheses) and add missing imports in examples.

Closes #153.

Copy link
Author

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

  • README’s CommonJS dynamic import example incorrectly uses top-level await in CJS; suggest wrapping in an async function or using .then().
  • Parser.md inconsistently documents the parse signature (parse(css) vs parse(css, options?)).
  • Some docs still mix CommonJS require with ESM despite the ESM-only requirement; standardize remaining examples in updated files (e.g., Stringify.md).
  • Overall changes align well with v7 (css-tree parser, walker registration, option removals), but a few inconsistencies remain that can confuse users.
Summary of changes
  • Switched CI badges in README to GitHub Actions and updated project description to reflect css-tree parsing and PostCSS-modeled nodes.
  • Updated requirements (Node >= 20.19.0, PostCSS >= 8.4.14) and emphasized ESM-only usage with many examples converted to import.
  • Clarified walker usage: helpers are not auto-registered; added registerWalkers(Container) across docs and examples.
  • Removed/marked legacy parse() options as forward-compat only; simplified Exports and Parser docs accordingly.
  • Corrected node-specific docs (Comment, Punctuation, Parentheses, Word) and example imports.
  • Various consistency and correctness fixes across docs and examples.

@charliecreates charliecreates bot removed the request for review from CharlieHelps November 3, 2025 14:53
@charliecreates charliecreates bot requested a review from CharlieHelps November 3, 2025 14:57
…ignature, and convert remaining Stringify examples to ESM
@charliecreates charliecreates bot force-pushed the ai-153-charlie-documentation-update branch from 55c945f to 887f581 Compare November 3, 2025 14:59
@charliecreates
Copy link
Author

charliecreates bot commented Nov 3, 2025

Pushed updates to address the review notes:

  • docs/README.md and README: replaced the CJS guidance to use require('postcss-values-parser') for Node >= 20.19.0 (removed the confusing top‑level await import).
  • docs/Parser.md: updated header to parse(css, options?) and noted that options are accepted for forward‑compat but ignored in v7.
  • docs/Stringify.md: converted the remaining require(...) examples to ESM import for consistency.

Reference: Node v20.19.0 enables require() for native ESM by default (no flag), so using require('postcss-values-parser') in CommonJS is valid on our minimum version. See the official release post: https://nodejs.org/en/blog/release/v20.19.0.

Copy link
Author

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

  • Parser docs still show ## parse(css) while other docs mention parse(css, options?); align the signature and clarify that options are ignored in v7.
  • docs/README.md uses top‑level await in a CommonJS note; replace with CJS‑safe examples (Promise/async wrapper) or the Node >= 20.19.0 require() approach.
  • Stringify.md retains some CommonJS require examples despite ESM‑only guidance; convert remaining snippets to import for consistency.
  • Overall, the ESM migration and walker registration guidance look good, but a few inconsistencies remain that could mislead users.
Summary of changes
  • Replaced CI badges with GitHub Actions in README and updated project description to reflect css-tree parsing and PostCSS-modeled nodes.
  • Updated requirements to Node.js >= 20.19.0 and PostCSS >= 8.4.14; emphasized ESM-only usage and added a CommonJS require() example in README.
  • Standardized many examples to ESM import syntax across docs; adjusted examples to include walker registration via registerWalkers(Container).
  • Simplified and clarified the parser docs: removed legacy options sections, added notes on forward-compatible options, and documented css-tree node mappings and source mapping behavior.
  • Cleaned up and clarified several node docs (Comment, Punctuation, Parentheses) and example imports.
  • Marked ParseOptions as a v7 placeholder and updated Exports to reflect current API.
  • Removed SCSS/LESS variable detection example and added a note that v7 only detects --* variables.

@charliecreates charliecreates bot requested review from CharlieHelps and removed request for CharlieHelps November 3, 2025 15:01
Copy link
Author

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

  • CommonJS guidance is inconsistent: README and docs/README prefer dynamic import() while the minimum Node.js version (>= 20.19.0) can require() ESM; standardize on require() for clarity.
  • Stringify docs conflict with v7 behavior by claiming whitespace preservation; update to state that spacing is normalized and adjust the example output.
  • Parser examples use walkFuncs without showing walker registration; add registerWalkers(Container) to avoid confusing users.
  • Minor: clarify walker registration in README Benefits to set expectations upfront.
Additional notes (1)
  • Readability | docs/Stringify.md:149-155
    The “Preserving Formatting” section claims that original spacing is preserved, but v7 docs elsewhere state that spacing is normalized and not preserved. This contradiction can cause misuse and incorrect expectations for toString() behavior.
Summary of changes
  • Switched README badges to GitHub Actions and updated the project description to reflect css-tree parsing with PostCSS-modeled nodes.
  • Updated Requirements to Node.js >= 20.19.0 and PostCSS >= 8.4.14; added an ESM-only note with CommonJS guidance.
  • Standardized many code examples to ESM import syntax across docs; added explicit walker registration via registerWalkers(Container) in multiple docs.
  • Aligned API docs: parse(css, options?) with options accepted for forward-compat in v7 but ignored; pruned legacy options from Exports/Parser and marked ParseOptions as a placeholder.
  • Clarified node docs (Comment, Punctuation, Parentheses, Word) and fixed/modernized example imports.
  • Removed SCSS/LESS variable examples and replaced with a note that Word.isVariable only detects --* in v7.

Comment on lines +191 to 193
root.walkFuncs((func) => {
console.log(`Function: ${func.name}`);
});
Copy link
Author

Choose a reason for hiding this comment

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

This example uses root.walkFuncs(...), but walker helpers are no longer auto-registered. Without registration, walkFuncs won’t be available, which may frustrate readers trying to follow the example.

Suggestion

Prepend walker registration to the example:

import { parse, registerWalkers, Container } from 'postcss-values-parser';
registerWalkers(Container);

const root = parse('calc(100% - 20px) url("bg.jpg") center/cover');
root.walkFuncs((func) => {
  console.log(`Function: ${func.name}`);
});

Reply with "@CharlieHelps yes please" if you'd like me to add a commit updating this example.

Copy link
Owner

Choose a reason for hiding this comment

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

@CharlieHelps yes please

Copy link
Author

Choose a reason for hiding this comment

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

Applied. Added walker registration to the example so walkFuncs works in v7.

Updated snippet
import { parse, registerWalkers, Container } from 'postcss-values-parser';

// Walker helpers are not auto-registered in v7
registerWalkers(Container);

const root = parse('calc(100% - 20px) url("bg.jpg") center/cover');
root.walkFuncs((func) => {
  console.log(`Function: ${func.name}`);
});

@charliecreates charliecreates bot removed the request for review from CharlieHelps November 3, 2025 15:05
@charliecreates charliecreates bot requested a review from CharlieHelps November 3, 2025 15:29
Copy link
Author

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

  • Stringify docs still claim original whitespace is preserved; v7 normalizes spacing. Update the section and example to reflect normalization.
  • Parser environment support lists “Node.js (all supported versions)” which conflicts with the new minimum of Node >= 20.19.0 and ESM-only. Align the statement with Requirements.
  • Consider adding a brief caveat in README and docs/README about require()-ing ESM on Node >= 20.19.0 being limited to synchronous ESM graphs (no top-level await).
  • Walker registration, ESM conversion, and parse(css, options?) consistency otherwise look solid.
Additional notes (3)
  • Compatibility | docs/Stringify.md:145-145
    This section asserts that original spacing is preserved, which conflicts with the v7 behavior you documented elsewhere (“spacing is normalized”). Keeping this claim will mislead users about toString() and the default stringifier output.

  • Readability | docs/Parser.md:215-215
    The environment support list says “Node.js (all supported versions)”, but the package now requires Node >= 20.19.0 and is ESM-only. This inconsistency can confuse users about runtime support.

  • Readability | docs/README.md:19-24
    Same as README: adding a brief caveat about require() limitations in CJS on Node >= 20.19.0 will reduce user confusion and support questions.

Summary of changes
  • Replaced legacy CI badges with GitHub Actions in README and refreshed project description to reflect css-tree parsing with PostCSS-modeled nodes.
  • Updated Requirements to Node.js >= 20.19.0 and PostCSS >= 8.4.14; documented ESM-only usage and added CommonJS require() guidance for Node >= 20.19.0.
  • Standardized many examples to ESM import; removed/clarified legacy parse() options and marked ParseOptions as a v7 placeholder.
  • Documented walker helper registration (registerWalkers(Container)) across README and multiple docs; removed claims of auto-registration.
  • Corrected node-specific docs (Comment, Punctuation, Parentheses, Word), refined Exports, and aligned Parser.md to parse(css, options?).
  • Minor copy and consistency edits across docs, plus example fixes (e.g., URL handling, function walking).

@charliecreates charliecreates bot removed the request for review from CharlieHelps November 3, 2025 15:35
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.

charlie: Documentation Update

3 participants