Create separate jekyll ts#11
Conversation
There was a problem hiding this comment.
Pull request overview
This PR creates a separate TypeScript setup specifically for Jekyll-related code, moving article-related TypeScript functionality from a previous location to a new assets/ts/ directory structure. The change introduces new build configuration, reorganizes file paths, and adds code formatting standards via Prettier.
Key changes:
- New TypeScript project setup in
assets/ts/with its own tsconfig.json and package.json - Migration of article.ts source code and compiled outputs to the new structure
- Updated path references in HTML layout to point to the reorganized assets
- Addition of Prettier configuration for code formatting consistency
Reviewed changes
Copilot reviewed 4 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
assets/ts/tsconfig.json |
New TypeScript compiler configuration for the Jekyll-specific TS project |
assets/ts/package.json |
New package configuration with TypeScript and ts-node dependencies |
assets/ts/package-lock.json |
Lock file for the new TypeScript project dependencies |
assets/ts/article.ts |
Migrated TypeScript source for article page functionality (syntax highlighting, reading time, scroll progress) |
assets/js/article.js |
Compiled JavaScript output with added comments and source map reference |
assets/js/article.js.map |
Source map for debugging TypeScript in browser |
assets/js/article.d.ts |
TypeScript declaration file generated from compilation |
assets/js/article.d.ts.map |
Declaration source map |
assets/css/article.css |
New CSS file for article styling (130 lines of comprehensive styles) |
_layouts/article.html |
Updated paths from /css/jekyll/article.css and /ts/dist/jekyll/article.js to new asset locations |
.prettierrc |
New Prettier configuration with 72-char line width and consistent formatting rules |
.prettierignore |
Ignore patterns for Prettier (node_modules, dist, build) |
Files not reviewed (1)
- assets/ts/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "description": "", | ||
| "main": "index.js", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" |
There was a problem hiding this comment.
The package.json is missing a build script to compile TypeScript to JavaScript. Add a "build" script like "tsc" or "tsc --watch" to the scripts section so developers know how to compile the TypeScript code.
| "test": "echo \"Error: no test specified\" && exit 1" | |
| "test": "echo \"Error: no test specified\" && exit 1", | |
| "build": "tsc" |
| "node_modules/typescript": { | ||
| "version": "5.9.3", | ||
| "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", | ||
| "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", | ||
| "dev": true, | ||
| "license": "Apache-2.0", | ||
| "peer": true, |
There was a problem hiding this comment.
TypeScript is marked as "peer": true in package-lock.json (line 204), which means it's only a peer dependency. However, it should be a direct devDependency as specified in package.json. This may indicate an issue with how npm install was run. Try removing package-lock.json and node_modules, then running npm install again to fix the dependency tree.
| "integrity": "sha512-gWEkeiyYE4vqjON/+Obqcoeffmk0NF15WSBwSs7zwVA2bAbTaE0SJ7P0WNGoJn8uE7fiaV5a7dKYIJriEqOrmA==", | ||
| "dev": true, | ||
| "license": "MIT", | ||
| "peer": true, |
There was a problem hiding this comment.
@types/node is marked as "peer": true in package-lock.json (line 92), which means it's only a peer dependency. However, it should be a direct devDependency as specified in package.json. This may indicate an issue with how npm install was run. Try removing package-lock.json and node_modules, then running npm install again to fix the dependency tree.
| "peer": true, |
| "isolatedModules": true, | ||
| "noUncheckedSideEffectImports": true, | ||
| "moduleDetection": "force", | ||
| "skipLibCheck": true, |
There was a problem hiding this comment.
Trailing comma after the last property in compilerOptions will cause a JSON parsing error. Remove the comma after "skipLibCheck": true.
| "skipLibCheck": true, | |
| "skipLibCheck": true |
|
|
||
| // Recommended Options | ||
| "strict": true, | ||
| "jsx": "react-jsx", |
There was a problem hiding this comment.
The jsx compiler option is set to "react-jsx", but there are no React dependencies in package.json and the code doesn't use JSX. This option should be removed unless React is intended to be added.
| "jsx": "react-jsx", |
| "target": "esnext", | ||
| "types": [], | ||
| // For nodejs: | ||
| // "lib": ["esnext"], |
There was a problem hiding this comment.
The types array is set to empty ([]), but the code uses DOM APIs like document and window. Without specifying DOM types, TypeScript should throw errors unless lib includes DOM types. Consider adding "lib": ["ESNext", "DOM"] to ensure proper type checking for browser APIs.
| // "lib": ["esnext"], | |
| "lib": ["ESNext", "DOM"], |
| // Environment Settings | ||
| // See also https://aka.ms/tsconfig/module | ||
| "module": "esnext", | ||
| "target": "esnext", |
There was a problem hiding this comment.
Using "target": "esnext" is not recommended for production code as it's a moving target that changes with each TypeScript version. Consider using a specific version like "ES2022" (as used in the main ts/tsconfig.json) for consistency and predictability.
| "target": "esnext", | |
| "target": "ES2022", |
|
|
||
| // Environment Settings | ||
| // See also https://aka.ms/tsconfig/module | ||
| "module": "esnext", |
There was a problem hiding this comment.
Using "module": "esnext" is not recommended for production code as it's a moving target. Consider using a specific module format like "ES2022" or "ESNext" spelled consistently. The main ts/tsconfig.json uses "ESNext" (capital N).
| "module": "esnext", | |
| "module": "ESNext", |
| "keywords": [], | ||
| "author": "", | ||
| "license": "ISC", | ||
| "type": "commonjs", |
There was a problem hiding this comment.
The tsconfig.json uses "module": "esnext" and "target": "esnext", but package.json specifies "type": "commonjs". This creates a mismatch where TypeScript will emit ES modules but Node.js will expect CommonJS. Either change the package.json type to "module" or adjust the TypeScript module setting to align with CommonJS.
| "type": "commonjs", | |
| "type": "module", |
No description provided.