Skip to content

Conversation

@ricokahler
Copy link

@ricokahler ricokahler commented Mar 31, 2025

Motivation

This PR introduces several tooling and structural improvements to enhance code quality, enforce documentation standards, and simplify the package's export configuration. These changes serve as preparatory work for the upcoming refactoring related to the synchronous evaluator, aiming for a cleaner and more maintainable codebase.

Key Changes:

  1. Enhanced Linting:

    • Added eslint-plugin-import-x and configured rules to enforce consistent import ordering, style, and prevent duplicates (import-x/order, import-x/consistent-type-specifier-style, import-x/no-duplicates, etc.).
    • Added eslint-plugin-tsdoc and enabled the tsdoc/syntax rule to validate TSDoc comment syntax, promoting better inline documentation.
    • Added general quality rules like no-console and no-warning-comments.
  2. Simplified Export Structure:

    • Removed the previous entry point src/1.ts.
    • Introduced a new centralized export file src/_exports/index.ts.
    • Updated package.json exports field:
      • The main "." entry point now correctly points to src/_exports/index.ts (source) and its corresponding dist files.
      • Removed the specific "./1" export entry.
    • Removed the typesVersions field from package.json as it's no longer needed after removing the specific ./1 export.
  3. API Extractor Configuration:

    • Disabled the ae-missing-release-tag rule in package.config.ts. TSDoc validation (@public, @internal within comments) will be the primary mechanism for controlling API surface visibility going forward.
  4. Dependency Updates:

    • Added eslint-plugin-import-x and eslint-plugin-tsdoc to devDependencies.
    • Updated pnpm-lock.yaml accordingly.

Impact

  • Improved code consistency through stricter linting, especially regarding imports.
  • Encourages better documentation practices via TSDoc validation.
  • Provides a clearer and more standard package export structure by using a single entry point file.
  • Removes the potentially confusing ./1 subpath export.

This PR primarily affects the developer experience and internal structure and should not introduce breaking changes for consumers using the main package entry point (import {...} from 'groq-js').

- Enhanced ESLint configuration by adding new rules and plugins, including `eslint-plugin-import-x` and `eslint-plugin-tsdoc`, to improve code quality and documentation.
- Updated `package.json` to reflect the addition of new dependencies and removed obsolete entries.
- Refactored the project structure by deleting the `src/1.ts` file and reorganizing exports to streamline the codebase.
- Introduced a new `markProcessor` and `parser` module to improve parsing capabilities and maintainability.
@ricokahler ricokahler marked this pull request as ready for review March 31, 2025 23:41
Copy link
Author

ricokahler commented Mar 31, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@socket-security
Copy link

socket-security bot commented Mar 31, 2025

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] Transitive: environment, filesystem +48 3.65 MB jounqin
npm/[email protected] Transitive: environment, eval, filesystem, unsafe +15 4.8 MB odspnpm

View full report↗︎

- Removed the 'ae-missing-release-tag' rule from the ESLint configuration.
- Added detailed JSDoc comments to various types and interfaces in `nodeTypes.ts`, `types.ts`, `evaluator/types.ts`, `parser/parser.ts`, `typeEvaluator/types.ts`, and `values/types.ts` to improve clarity and public API documentation.
- Updated the `parse` function in `parser.ts` to include a public annotation for better visibility in generated documentation.
@ricokahler ricokahler changed the title chore: update ESLint configuration and dependencies; restructure repo chore: Improve linting, docs, and export structure Apr 1, 2025
@ricokahler ricokahler changed the title chore: Improve linting, docs, and export structure chore: improve linting, docs, and export structure Apr 1, 2025
@ricokahler ricokahler requested a review from judofyr April 1, 2025 21:54
Copy link
Collaborator

@judofyr judofyr left a comment

Choose a reason for hiding this comment

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

This now needs a rebase. It would also be good to split it into separate PRs and not change everything at once.

"type": "commonjs",
"exports": {
".": {
"source": "./src/1.ts",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason why you removed this? The intention of this was to be compatible with new breaking changes of GROQ (the language). In that case we would implement a new export (e.g. "groq-js/2" so that groq-js can still implement version 1 of GROQ through a separate export. The intention of using exports here was in case we would do more drastic changes to e.g. the parser and we would like to keep completely separate versions (and thus improve the possibility of tree shaking).

That said, it's probably not to critical to not have this now. Once we break GROQ we can always release a new minor of groq-js which introduces groq-js/2 and groq-js/1 (but keeps groq-js defaulting to groq-js/1).

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.

2 participants