-
Notifications
You must be signed in to change notification settings - Fork 1
Add TypeScript package generator with PlopJS #667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 5da6ccb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
UHM isn't this the goal of our cli? why making another package? |
shouldn't the cli use plop generators to generate workspaces? |
packages/typescript-generator/templates/typescript-package/.node-version.hbs
Outdated
Show resolved
Hide resolved
packages/typescript-generator/templates/typescript-package/eslint.config.js.hbs
Outdated
Show resolved
Hide resolved
packages/typescript-generator/templates/typescript-package/package.json.hbs
Outdated
Show resolved
Hide resolved
packages/typescript-generator/templates/typescript-package/package.json.hbs
Show resolved
Hide resolved
packages/typescript-generator/templates/typescript-package/package.json.hbs
Outdated
Show resolved
Hide resolved
"build": "tsup", | ||
"dev": "tsup --watch", | ||
"typecheck": "tsc --noEmit", | ||
"lint": "eslint --fix src", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make the setup as easier as possible - IMHO there are no reason for pushing to our consumers a code bundler. tsc
is easy to use and it is good enough for the majority of use case we have.
(I would include esbuild/tsup
only for advanced setup)
"build": "tsup", | |
"dev": "tsup --watch", | |
"typecheck": "tsc --noEmit", | |
"lint": "eslint --fix src", | |
"build": "tsc", | |
"build:watch": "tsc --watch", | |
"typecheck": "tsc --noEmit", | |
"lint": "eslint --fix src", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've included tsup in order to produce both esm and commonjs packages for publication. do you suggest to do the same using two different tsconfig.json
or to use a bundler only within the publish pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we are going to use tsup, I think we should add it to the tech radar as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kin0992 yes for libraries I think is the best option, maybe we can try to use esbuild
directly (tsup
is just a thin wrapper of it) and add the latter to the technology radar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, my point is to use technologies that are listed in the technology radar (at least in the asses or trial section). what do you think?
packages/typescript-generator/templates/typescript-package/tsconfig.json.hbs
Outdated
Show resolved
Hide resolved
packages/typescript-generator/templates/typescript-package/tsconfig.json.hbs
Outdated
Show resolved
Hide resolved
packages/typescript-generator/templates/typescript-package/vitest.config.ts.hbs
Show resolved
Hide resolved
Co-authored-by: Luca Cavallaro <[email protected]>
…onstructed from library input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…a/dx into feats/typescript-generator
…kage/package.json.hbs Co-authored-by: Copilot <[email protected]>
…kage.json.hbs Co-authored-by: Copilot <[email protected]>
- Resolved conflicts in package.json and pnpm-workspace.yaml - Regenerated pnpm-lock.yaml using pnpm install to avoid lock file conflicts - Combined catalog entries from both branches appropriately
Keep file locally but remove from version control
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a comprehensive PlopJS generator system for creating TypeScript packages in the DX monorepo. The implementation provides a modular, extensible foundation for package generation with three distinct packages: a core shared library, a base generator, and a library-specific generator that adds build tooling.
- Adds PlopJS integration with a root-level plopfile and npm script
- Creates a shared core package (@pagopa/typescript-generator) for common generator functionality
- Implements two specific generators: a base TypeScript package generator and a library generator with tsup build configuration
Reviewed Changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
turbo.json | Adds build dependency to typecheck task |
pnpm-workspace.yaml | Adds PlopJS and inquirer dependencies to catalog |
plopfile.js | Root-level configuration for PlopJS generators |
packages/typescript-generator/* | Core shared functionality for TypeScript package generators |
packages/typescript-base-generator/* | Basic TypeScript package generator without additional build tools |
packages/typescript-library-generator/* | TypeScript library generator with tsup build configuration |
package.json | Adds generate script for running PlopJS |
.changeset/add-plop-generator.md | Changeset documenting the new generator functionality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also a rebase is required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: why not a .ts
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: I see 3 packages, but just one changelog. I'm not sure we do not want to track the changes on other packages.
@@ -0,0 +1,50 @@ | |||
{ | |||
"name": "@pagopa/typescript-base-generator", | |||
"version": "0.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: this will become 0.2.0
when changesets updates the version (because of the minor
bump).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment is valid for other packages as well
"@tsconfig/node22": "catalog:", | ||
"@types/node": "catalog:", | ||
"eslint": "catalog:", | ||
"prettier": "catalog:", | ||
"tsup": "catalog:", | ||
"typescript": "catalog:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: I think some of these need to be updated after some changes were introduced on main
.
e.g. eslint
should be imported from catalog:dx
When using in the DX monorepo, you can run the generator directly from the root directory: | ||
|
||
```bash | ||
pnpm run turbo gen typescript-base-generator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: are you sure this is working?
@@ -0,0 +1,31 @@ | |||
{ | |||
"name": "{{packageName}}", | |||
"version": "0.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
"devDependencies": {}, | ||
"scripts": { | ||
"start": "node .", | ||
"build": "tsup", | ||
"dev": "tsup --watch", | ||
"typecheck": "tsc --noEmit", | ||
"lint": "eslint --fix src", | ||
"lint:check": "eslint src", | ||
"format": "prettier --write .", | ||
"format:check": "prettier --check .", | ||
"test": "vitest run", | ||
"test:watch": "vitest", | ||
"test:coverage": "vitest run --coverage" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -0,0 +1,50 @@ | |||
{ | |||
"name": "@pagopa/typescript-library-generator", | |||
"version": "0.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
"@pagopa/eslint-config": "workspace:^", | ||
"@tsconfig/node22": "catalog:", | ||
"@types/node": "catalog:", | ||
"eslint": "catalog:", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
When using in the DX monorepo, you can run the generator directly from the root directory: | ||
|
||
```bash | ||
pnpm run turbo gen typescript-library-generator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
I think the right command should be pnpm exec turbo gen typescript-library-package
.
If you prompt typescript-library-generator
, the CLI is asking you to select the generator to use; options are typescript-package
and typescript-library-package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why exec turbo gen
and not directly pnpm turbo gen
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's ok pnpm turbo gen
as well
Introduce a PlopJS generator for creating TypeScript workspaces in the DX monorepo.