Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Add types to config-array #3

Merged
merged 8 commits into from
May 8, 2024
Merged

feat: Add types to config-array #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7c431a3
fix(types): Ensure CommonJS build works correctly
nzakas Apr 24, 2024
808f96e
feat: Add types to config-array
nzakas Apr 24, 2024
0f9be98
Add back prepare task
nzakas Apr 26, 2024
f721748
Exclude types.ts from package
nzakas May 6, 2024
f250890
Update packages/config-array/package.json
nzakas May 7, 2024
210e8fc
Add build script
nzakas May 7, 2024
ed3a043
Remove prepare script to allow CI to work
nzakas May 7, 2024
453377d
Add build step to release flow
nzakas May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ jobs:
- name: npm install and test
run: |
npm install
npm run build
npm test
env:
CI: true
12 changes: 5 additions & 7 deletions .github/workflows/release-please.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ jobs:
node-version: lts/*
registry-url: "https://registry.npmjs.org"

- run: npm install
- run: |
npm install
npm run build
if: ${{ steps.release.outputs.releases_created }}

#-----------------------------------------------------------------------------
Expand All @@ -48,9 +50,7 @@ jobs:
NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}

- name: Publish @eslint/object-schema package to JSR
run: |
npm run build --if-present
npx jsr publish
run: npx jsr publish
working-directory: packages/object-schema
if: ${{ steps.release.outputs['packages/object-schema--release_created'] }}

Expand Down Expand Up @@ -81,9 +81,7 @@ jobs:
NODE_AUTH_TOKEN: ${{secrets.NPM_TOKEN}}

- name: Publish @eslint/config-array package to JSR
run: |
npm run build --if-present
npx jsr publish
run: npx jsr publish
working-directory: packages/config-array
if: ${{ steps.release.outputs['packages/config-array--release_created'] }}

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"type": "module",
"scripts": {
"test": "npm test --workspaces --if-present",
"build": "node scripts/build.js",
"lint": "eslint .",
"lint:fix": "eslint --fix .",
"fmt": "prettier --write ."
Expand Down
9 changes: 7 additions & 2 deletions packages/config-array/jsr.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@
"version": "0.12.3",
"exports": "./dist/esm/index.js",
"publish": {
"exclude": [
"!dist"
"include": [
"dist/esm/index.js",
"dist/esm/index.d.ts",
"dist/esm/types.d.ts",
"README.md",
"jsr.json",
"LICENSE"
]
}
}
20 changes: 16 additions & 4 deletions packages/config-array/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@
"default": "./dist/esm/index.js"
}
},
"files": [
"dist/cjs/index.cjs",
"dist/cjs/index.d.cts",
"dist/cjs/types.d.ts",
"dist/esm/index.js",
"dist/esm/index.d.ts",
"dist/esm/types.d.ts",
"README.md",
"LICENSE"
],
"repository": {
"type": "git",
"url": "git+https://github.com/eslint/rewrite.git"
Expand All @@ -23,8 +33,9 @@
},
"homepage": "https://github.com/eslint/rewrite#readme",
"scripts": {
"build": "rollup -c",
"prepare": "npm run build",
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
"build:dedupe-types": "node ../../tools/dedupe-types.js dist/cjs/index.cjs dist/esm/index.js",
"build": "rollup -c && npm run build:dedupe-types && tsc -p tsconfig.esm.json && tsc -p tsconfig.cjs.json",
Copy link
Member

@kecrily kecrily Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We need to either delete the copied dist/{esm,cjs}/types.ts at the end of the build, or rename it to .d.ts when we copy it to avoid generating declarations of statements.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I'm not sure I follow. This outputs both types.ts and types.d.ts in the output directories, unless I'm missing something?

Copy link
Member

@kecrily kecrily May 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, types.ts is not required at publish time. And the contents of types.ts and types.d.ts are the same.

Snapshot 2024-05-05 22 14 57

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha. I think we can solve that by using files in package.json to avoid adding another step to this process.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. I think we can solve that by using files in package.json to avoid adding another step to this process.

Should we then do the same in the object-schema package?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'll update all the packages in a separate PR.

"pretest": "npm run build",
"test": "mocha tests/"
},
"keywords": [
Expand All @@ -39,10 +50,11 @@
"minimatch": "^3.0.5"
},
"devDependencies": {
"@types/minimatch": "^3.0.5",
"mocha": "^10.4.0",
"rollup": "^4.16.2",
"typescript": "^5.4.5",
"rollup-plugin-copy": "^3.5.0"
"rollup-plugin-copy": "^3.5.0",
"typescript": "^5.4.5"
},
"engines": {
"node": "^18.18.0 || ^20.9.0 || >=21.1.0"
Expand Down
23 changes: 17 additions & 6 deletions packages/config-array/src/base-schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,27 @@
* @author Nicholas C. Zakas
*/

//------------------------------------------------------------------------------
// Types
//------------------------------------------------------------------------------

/** @typedef {import("@eslint/object-schema").PropertyDefinition} PropertyDefinition */
/** @typedef {import("@eslint/object-schema").ObjectDefinition} ObjectDefinition */

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
* A strategy that does nothing.
* @type {PropertyDefinition}
*/
const NOOP_STRATEGY = {
required: false,
merge() {
return undefined;
},
validate() { }
validate() {},
};

//------------------------------------------------------------------------------
Expand All @@ -21,7 +32,7 @@ const NOOP_STRATEGY = {

/**
* The base schema that every ConfigArray uses.
* @type Object
* @type {ObjectDefinition}
*/
export const baseSchema = Object.freeze({
name: {
Expand All @@ -30,11 +41,11 @@ export const baseSchema = Object.freeze({
return undefined;
},
validate(value) {
if (typeof value !== 'string') {
throw new TypeError('Property must be a string.');
if (typeof value !== "string") {
throw new TypeError("Property must be a string.");
}
}
},
},
files: NOOP_STRATEGY,
ignores: NOOP_STRATEGY
ignores: NOOP_STRATEGY,
});
Loading