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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: do not publish ts files to npm #21

Closed
wants to merge 3 commits into from
Closed

Conversation

aladdin-add
Copy link
Member

@aladdin-add aladdin-add commented May 13, 2024

this is to exclude ts files when publishing to npm.

npm:

npm pack -w packages/compat
npm notice 
npm notice 馃摝  @eslint/compat@1.0.1
npm notice === Tarball Contents === 
npm notice 11.4kB LICENSE            
npm notice 3.5kB  README.md          
npm notice 7.9kB  dist/cjs/index.cjs 
npm notice 776B   dist/cjs/types.d.ts
npm notice 1.6kB  dist/esm/index.d.ts
npm notice 7.8kB  dist/esm/index.js  
npm notice 776B   dist/esm/types.d.ts
npm notice 1.4kB  package.json       
npm notice === Tarball Details === 
npm notice name:          @eslint/compat                          
npm notice version:       1.0.1                                   
npm notice filename:      eslint-compat-1.0.1.tgz                 
npm notice package size:  8.2 kB                                  
npm notice unpacked size: 35.1 kB                                 
npm notice shasum:        71b147013f318a1e0e3daa664aed6f720700a9e3
npm notice integrity:     sha512-5eK8Rx8ZyPWpr[...]Rr192v0UilFXA==
npm notice total files:   8                                       
npm notice 
eslint-compat-1.0.1.tgz

refs: #16 (comment)

jsr:

Checking for slow types in the public API...
Simulating publish of @eslint/compat@1.0.1 with files:
   file:///Users/weiran/repo/github/eslint-rewrite/packages/compat/LICENSE (11.09KB)
   file:///Users/weiran/repo/github/eslint-rewrite/packages/compat/README.md (3.41KB)
   file:///Users/weiran/repo/github/eslint-rewrite/packages/compat/dist/esm/index.d.ts (1.53KB)
   file:///Users/weiran/repo/github/eslint-rewrite/packages/compat/dist/esm/index.js (7.66KB)
   file:///Users/weiran/repo/github/eslint-rewrite/packages/compat/dist/esm/types.d.ts (776B)
   file:///Users/weiran/repo/github/eslint-rewrite/packages/compat/dist/esm/types.ts (982B)
   file:///Users/weiran/repo/github/eslint-rewrite/packages/compat/jsr.json (247B)
Warning Aborting due to --dry-run

Completed in 14s

this is to exclude ts files when publishing to npm.
@@ -14,7 +14,7 @@
}
},
"files": [
"dist"
"dist/**/*.{js,cjs,mjs,d.ts}"
Copy link
Member Author

Choose a reason for hiding this comment

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

mjs was not used; it was included in case there may be some mjs files later.

Copy link
Member

Choose a reason for hiding this comment

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

Screenshot 2024-05-13 at 1 53 52 PM

I can see ts imports from declarations file. I dont think ignoring it would work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not see this line; have you pulled the latest changes?

just pushed a commit: 3baee87

Copy link
Member

Choose a reason for hiding this comment

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

We still would need the ts file after resolving to .js the import inside the d.ts is actually resolving to types.ts and not types.js. Try looking at the compat build files once 馃

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, good catch! seems it's always required (not only jsr as described in #16 (comment)).

@aladdin-add aladdin-add changed the title chore: do not publish ts files chore: do not publish ts files to npm May 13, 2024
@aladdin-add aladdin-add marked this pull request as draft May 13, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

None yet

2 participants