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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Add build/.eslintrc.json to the files field in package.json #553

Merged
merged 1 commit into from
Oct 8, 2020

Conversation

pokutuna
Copy link
Contributor

@pokutuna pokutuna commented Aug 25, 2020

There is .eslintrc.json in build/ directory that is generated by tsc. This file is loaded from build/src/index.js but not listed in the files field.

This is already included in published tarballs.

$ curl -s https://registry.npmjs.org/gts/-/gts-2.0.2.tgz | tar tv | grep .eslintrc.json
   -rw-rw-r--  0 0      0        1407 10 26  1985 package/.eslintrc.json
   -rw-rw-r--  0 0      0        1663 10 26  1985 package/build/.eslintrc.json

This pr not only makes this package correct.
To add this file makes us able to use the head version from the GitHub repository, using yarn add -D gts@git+https://github.com/google/gts.git#master.

In installing from git dependency, the prepare script runs and compiles typescript files. But now it doesn't work, yarn copies files listed in the files to under node_modules from cache directory where this compiled.
(But failed on npm. I guess npm is failing to execute chmod to set permissions for bin files not compiled yet.)

@google-cla google-cla bot added the cla: yes label Aug 25, 2020
180254 added a commit to 180254/Battleship44 that referenced this pull request Sep 21, 2020
google/gts#553

Signed-off-by: Adrian Pedziwiatr <pedziwiatrad@gmail.com>
@pokutuna
Copy link
Contributor Author

pokutuna commented Oct 8, 2020

Can someone please review this?
This change is tiny & based on the actual package.

src/index.ts will be placed at build/src/index.js after compiling, and loads build/.eslintrc.json

import * as cfg from '../.eslintrc.json';

I agree it's strange that files have 2 .eslintrc.json.

@JustinBeckwith JustinBeckwith changed the title Add build/.eslintrc.json to the files field in package.json fix: Add build/.eslintrc.json to the files field in package.json Oct 8, 2020
Copy link
Collaborator

@JustinBeckwith JustinBeckwith left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and apologies for the delay. Agreed this is super weird, but looking at index.ts by golly you're right. I think this actually explains a few bugs I've seen 😆

@JustinBeckwith JustinBeckwith merged commit 3b516ad into google:master Oct 8, 2020
nevilm-lt pushed a commit to nevilm-lt/gts that referenced this pull request Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants