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

Update dependencies #928

Merged
merged 4 commits into from
Dec 1, 2023
Merged

Update dependencies #928

merged 4 commits into from
Dec 1, 2023

Conversation

smaye81
Copy link
Member

@smaye81 smaye81 commented Dec 1, 2023

This prepares for the v1.2.0 release by accounting for some issues that arose during the release process. Since the process removes package-lock.json and causes deps to be updated, some of the updated deps are causing the build to fail.

To prepare, the package-lock.json and node_modules directory were removed and npm install was run. Then, make was run to discover any issues, which were as follows:

  • @typescript/eslint added a new rule in v6.11.0 which fails when comparing an enum type against a different type. This causes a function in connect-node to fail. So, the rule has been disabled for that function.

  • karma-esbuild is somehow broken when updating it alongside karma and esbuild. Since Karma itself is deprecated and karma-esbuild sat stagnant for over a year (until the latest release which is causing the issues), it's probably best to just pin to the one on main until we can move off of Karma asap.

@smaye81 smaye81 requested a review from timostamm December 1, 2023 00:41
@smaye81
Copy link
Member Author

smaye81 commented Dec 1, 2023

@timostamm not sure how you want to handle these issues. I took a stab at mitigating them, but if you have other ideas, let me know. I put this in its own PR just so it was easier to evaluate the fixes without muddying up the release PR.

Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

karma-esbuild is somehow broken when updating it alongside karma and esbuild.

This seems odd, since the karma-esbuild release has been out for a while without new issue reports, and there have been forks as well.

It would be helpful to add the exact error message here in cases like this, so that others who might know more about the issue can chime in.

Was it this error?

ERROR [plugin]: Cannot load "esbuild", it is not registered!
Perhaps you are missing some plugin?

Karma auto-registers plugins from packages in node_modules when their name starts with "karma-". If karma-esbuild is installed nested in a workspace, karma installed from the root package won't find it. If I make sure that karma-esbuild is in the top level node_modules next to karma, make testwebbrowser passes for me.

Please give this another try, but make sure to update esbuild everywhere, so that karma-esbuild is hoisted into the root node_modules.

packages/connect-node/src/node-error.ts Outdated Show resolved Hide resolved
packages/connect-web-bench/README.md Outdated Show resolved Hide resolved
@timostamm
Copy link
Member

Can you name this PR "Update dependencies"?

@smaye81 smaye81 changed the title Prepare for v1.2.0 release Update dependencies Dec 1, 2023
@smaye81
Copy link
Member Author

smaye81 commented Dec 1, 2023

@timostamm incorporated your feedback. Lmk how it looks.

Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

LGTM, but please remove the unused linter comment.

packages/connect-node/src/node-error.ts Outdated Show resolved Hide resolved
@smaye81 smaye81 merged commit 22ef9ed into main Dec 1, 2023
3 checks passed
@smaye81 smaye81 deleted the sayers/prerelease_1.2.0 branch December 1, 2023 16:36
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.

2 participants