Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive TypeScript support and improves the project configuration for the opencc-wasm library. The changes include type definitions, enhanced gitignore patterns, and updated package.json to properly reference the new TypeScript types and build outputs.
- Added complete TypeScript type definitions with JSDoc documentation
- Updated package.json to reference new ESM/CJS build locations and include TypeScript types
- Enhanced .gitignore with comprehensive exclusions for node_modules, logs, OS files, and editor configurations
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| wasm-lib/package.json | Updated module entry points to new dist/esm and dist/cjs paths, added TypeScript types field, and included LICENSE/NOTICE in published files |
| wasm-lib/index.d.ts | New TypeScript type definitions file providing complete type safety with interfaces for ConverterOptions, ConverterFunction, OpenCCNamespace, and related types with JSDoc examples |
| wasm-lib/.gitignore | Expanded from single line to comprehensive exclusions covering node_modules, logs, OS files, editor configs, and cache directories |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c63da4c to
37fc54d
Compare
…eparation This commit enhances the opencc-wasm library with TypeScript support and implements a cleaner build architecture with semantic separation between intermediate build artifacts and publishable distribution. TypeScript Support: - Add comprehensive type definitions (index.d.ts) with full JSDoc documentation - Define interfaces: ConverterOptions, ConverterFunction, OpenCCNamespace, etc. - Provide complete type safety for better IDE support and developer experience Build Architecture Redesign (semantic separation): - build/ - Intermediate WASM artifacts (gitignored, for tests/development) * build/opencc-wasm.esm.js - ESM WASM glue * build/opencc-wasm.cjs - CJS WASM glue * build/opencc-wasm.wasm - WASM binary - dist/ - Publishable distribution (committed, for npm) * dist/esm/ - ESM package entry * dist/cjs/ - CJS package entry * dist/data/ - OpenCC config and dictionary files Invariants and Semantics: - Tests import source (index.js) → loads from build/ - Published package exports dist/ only - build/ = internal intermediate artifacts - dist/ = publishable artifacts - Clear separation ensures tests validate actual build output Enhanced .gitignore: - Add build/ to gitignore (intermediate artifacts) - Add node_modules/, logs, OS-specific files (.DS_Store, Thumbs.db) - Exclude editor configurations (.vscode/, .idea/) - Add cache and temporary file exclusions Two-Stage Build Process: Stage 1 (build.sh): - Compiles C++ to WASM using Emscripten - Outputs to build/ directory Stage 2 (build-api.js): - Copies WASM artifacts from build/ to dist/ - Transforms source paths for production - Generates API wrappers for ESM and CJS - Copies data files Package Configuration (package.json): - Add "types" field pointing to index.d.ts - Update "main" and "module" to point to API wrappers in dist/ - Add comprehensive "exports" map: * "." - Main API (ESM/CJS wrappers) * "./wasm" - Direct access to WASM glue for advanced users * "./dist/*" - Wildcard for flexible file access - Include LICENSE and NOTICE in published files Documentation: - Add comprehensive README section explaining build architecture - Document project structure with invariants - Explain semantic separation between build/ and dist/ Benefits: - Better TypeScript integration and IDE autocomplete - Cleaner, more maintainable directory structure - Tests validate actual build output, not stale dist files - Clear semantic separation between internal and publishable artifacts - Professional project setup following modern npm best practices - Long-term maintainability through clear invariants
37fc54d to
7782d2e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Build intermediates (not committed) | ||
| build/ | ||
|
|
||
| # Dist outputs (keep in git for npm package) |
There was a problem hiding this comment.
The generated dist files (opencc-wasm.js and opencc-wasm.cjs) are minified production bundles that should not be committed to git as they are binary-like artifacts that create merge conflicts and bloat the repository. According to the PR description, build/ is gitignored for intermediate artifacts, but dist/ should contain publishable outputs. However, these specific Emscripten-generated WASM glue files are large minified files (likely auto-generated) that should be treated as build outputs.
Consider either:
- Adding dist/esm/opencc-wasm.js and dist/cjs/opencc-wasm.cjs to .gitignore (they can be generated during npm prepare/publish hooks)
- If they must be committed, document why in the README
| # Dist outputs (keep in git for npm package) | |
| # Dist outputs (keep in git for npm package) | |
| # (except specific Emscripten-generated WASM glue files) | |
| dist/esm/opencc-wasm.js | |
| dist/cjs/opencc-wasm.cjs |
This commit enhances the opencc-wasm library with TypeScript support and
implements a cleaner build architecture with semantic separation between
intermediate build artifacts and publishable distribution.
TypeScript Support:
Build Architecture Redesign (semantic separation):
Invariants and Semantics:
Enhanced .gitignore:
Two-Stage Build Process:
Stage 1 (build.sh):
Stage 2 (build-api.js):
Package Configuration (package.json):
Documentation:
Benefits: