-
-
Notifications
You must be signed in to change notification settings - Fork 207
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: Update dependencies to avoid build failure #631
Conversation
I see only ESLint updated in package.json. |
Oops, looks like I missed a commit. |
Now this is super weird, everything locally is working but the CI isn't. |
Yeah, I'm getting install errors before being able to look at tests. Did you do a fresh install at root? |
When I amend the |
Yeah, I've tried removing |
Even weirder is that when I remove that from the install script, the install proceeds and completes, saying that typescript 5.6.2 is installed but typescript is not present in |
I'm also seeing that the reported I can find any evidence of any package installing 4.5.5, though. |
The different set of packages in "workspaces": [
"packages/*"
], ...though I know that's the whole point. |
Okay, I think I know what's happening. The packages are installed in alphabetical order, so something in |
Well, different error, but old |
Could this be of significance? After running
Likewise do I get the error when installing within the directory. |
|
Oddly, when upgrading tsd, I get an even older version of typescript showing up within package.json |
The TS version is fixed when I change this in the root package.json: "workspaces": [
"packages/eslint-scope",
"packages/eslint-visitor-keys",
"packages/eslint-espree"
], |
The top-level So I think now I can ask you about the failing test in |
Ok, I can look into it, but it may be working on CI merely because it has a higher global TS version installed. With the change I made, the warnings like this are now gone also:
|
I'm afraid this was a bit fragile to begin with (at least incomplete), and now with updates, the script can't accommodate. I think you'll either need to build the keys again manually or find someone with greater bandwidth than myself to adapt the script. I'm finding it difficult to revisit the code unfortunately. |
@brettz9 okay, thanks. |
I've gone through and remove the tools because they don't work anymore. I think we're okay to update the keys manually when needed. |
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.
LGTM, thanks!
Prerequisites checklist
What is the purpose of this pull request?
What changes did you make? (Give an overview)
Upgraded the following packages to allow
npm install
to work without error:Related Issues
refs #630
Is there anything you'd like reviewers to focus on?
@brettz9 can you help me fix the failing test? I can't for the life of me figure out what it's trying to do, but it appears to not be handling
TSArrayType
s properly.