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

Run npx eslint . --ext .js,.ts --fix #277

Closed
wants to merge 1 commit into from

Conversation

silverwind
Copy link

@silverwind silverwind commented Mar 14, 2024

npm run build refused to proceed building without this.

I think the issue is because some eslint module is not properly locked down to a particular version. It would be even better to not have linting as a dependency to the build.

@silverwind silverwind requested a review from a team as a code owner March 14, 2024 22:21
@silverwind silverwind changed the title Run eslint . --ext .js,.ts --fix Run npx eslint . --ext .js,.ts --fix Mar 14, 2024
@silverwind
Copy link
Author

silverwind commented Mar 15, 2024

Hmm, the CI now suggests the inverse. There must be something wrong with my setup I suppose. Will try a fresh clone.

@silverwind
Copy link
Author

silverwind commented Mar 15, 2024

Nope, still present on fresh clone. Very strange. Node 21.7.1, npm 10.5.0.

$ npm i
added 583 packages in 34s
$ npm run build
> @github/relative-time-element@0.0.0-development prebuild
> npm run clean && npm run lint && mkdir dist


> @github/relative-time-element@0.0.0-development clean
> rm -rf dist


> @github/relative-time-element@0.0.0-development lint
> eslint . --ext .js,.ts && tsc --noEmit


/Users/silverwind/git/relative-time-element/src/duration-format-ponyfill.ts
  111:11  error  Insert `··`  prettier/prettier
  112:1   error  Insert `··`  prettier/prettier

/Users/silverwind/git/relative-time-element/src/duration.ts
  5:20  error  Replace `typeof·unitNames` with `(typeof·unitNames)`  prettier/prettier

/Users/silverwind/git/relative-time-element/src/relative-time-element.ts
   2:67  error  Replace `typeof·window` with `(typeof·window)`                                                                                                                                                                                           prettier/prettier
  14:15  error  Replace `public·oldText:·string,·public·newText:·string,·public·oldTitle:·string,·public·newTitle:·string` with `⏎····public·oldText:·string,⏎····public·newText:·string,⏎····public·oldTitle:·string,⏎····public·newTitle:·string,⏎··`  prettier/prettier

✖ 5 problems (5 errors, 0 warnings)
  5 errors and 0 warnings potentially fixable with the `--fix` option.

@keithamus
Copy link
Member

What is your git rev-parse HEAD?

@silverwind
Copy link
Author

silverwind commented Mar 15, 2024

1a42a317372e09b08fe430fe36f2598d2039bbfa, tip of main branch. Also tried another machine, same result. Maybe something about node 21, CI still uses node 18.

@keithamus
Copy link
Member

I'm unable to repro. I wonder if you have one of these tools installed globally which is overriding the local version or something? Whats the output of npm ls -g prettier or npm ls -g eslint?

@silverwind
Copy link
Author

Yes, I have many global packages. Will try to uninstall. Meanwhile I tested in a fresh docker container with node 21, and it does work in there, so this must be some npm global influence.

@silverwind
Copy link
Author

silverwind commented Mar 15, 2024

Found the problem. My ~/.npmrc contains lockfile-version=3, but this package has lockfileVersion=2, which is a legacy format that contains two entries for each module, while version 3 only contains one, so likely npm installed different versions for some dependencies than are in the lockfile.

Checking whether I can remove this without causing npm to write new lockfiles in v2, which I want to avoid...

@silverwind silverwind closed this Mar 15, 2024
@silverwind silverwind deleted the format branch March 15, 2024 10:42
@silverwind
Copy link
Author

silverwind commented Mar 15, 2024

Yeah, npm v10.5.0 still writes new v3 lockfiles, so the option was unnecessary and I removed it. Sorry for the trouble. The problem could have been avoided by declaring lockfileVersion in .npmrc in the project, but I'm not convinced this is really worth the effort.

@keithamus keithamus mentioned this pull request Mar 15, 2024
@silverwind
Copy link
Author

silverwind commented Mar 15, 2024

After testing #278, it turns out I made an error (I had moved away the ~/.npmrc and was not cleanly reinstalling node_modules). The actual cause in my ~/.npmrc was package-lock=false which made npm ignore the lockfile entirely. I'm suprised this option hadn't bitten me sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants