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

Dependency updates #128

Merged
merged 7 commits into from
May 14, 2024
Merged

Dependency updates #128

merged 7 commits into from
May 14, 2024

Conversation

wilwade
Copy link
Member

@wilwade wilwade commented May 14, 2024

Updated almost all of the dependencies to the latest version.

  • Bson updates required a few small changes due to es module updates in bson
  • Did not update chai past v4. Looks like moving past chai v4 would require many changes and not all the chai plugins we are using support it yet. Perhaps we should just move away from chai instead.
  • Moved from ts-node to tsx
  • Updates for msw breaking changes, but nothing special for our use cases.
  • Github actions with node no longer needs a separate cache step

Suggested Tests

  • Locally build and run tests
  • Try it out with:
    • npm i && npm run build && npm run serve
    • cd examples/server && npm i && node app.js
    • Then go to http://localhost:3000, open up the console and hit some buttons.

with:
node-version: '18.18.2'

- uses: actions/cache@v2
Copy link
Member Author

Choose a reason for hiding this comment

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

actions/setup-node@v4 has this included

registry-url: 'https://registry.npmjs.org'

- uses: actions/cache@v2
Copy link
Member Author

Choose a reason for hiding this comment

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

actions/setup-node@v4 has this included

registry-url: 'https://registry.npmjs.org'

- uses: actions/cache@v2
Copy link
Member Author

Choose a reason for hiding this comment

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

actions/setup-node@v4 has this included

@@ -0,0 +1,4 @@
{
"$schema": "https://json.schemastore.org/mocharc.json",
"require": "tsx"
Copy link
Member Author

Choose a reason for hiding this comment

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

Switching to tsx only requires this!

@@ -1,5 +1,5 @@
//
// Autogenerated by Thrift Compiler (0.16.0)
// Autogenerated by Thrift Compiler (0.18.1)
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 regen these files with v0.18.1, but nothing was different except this header.

Comment on lines +6 to +8
// BSON uses top level awaits, so use require for now
const bsonSerialize = require('bson').serialize;
const bsonDeserialize = require('bson').deserialize;
Copy link
Member Author

Choose a reason for hiding this comment

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

Really annoying and only impacts esbuild for the browser, but not too bad.

Why the double require? It was clearer than the named version and keeps the linter away from trying to reformat it.

Copy link
Member Author

Choose a reason for hiding this comment

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

These were updates for the new version of msw. No big changes however

@@ -12,15 +12,11 @@
"sourceMap": false,
"strict": true,
"target": "ESNext",
"watch": false,
Copy link
Member Author

Choose a reason for hiding this comment

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

This never did anything.

Comment on lines -23 to -25
"ts-node": {
"files": true
}
Copy link
Member Author

Choose a reason for hiding this comment

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

No more ts-node

Copy link
Collaborator

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

I got this error on cd examples && npm i && node app.js:

node:internal/modules/cjs/loader:1080
  throw err;
  ^

Error: Cannot find module '/Users/shannonwells/github/parquetjs/examples/app.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
    at Module._load (node:internal/modules/cjs/loader:922:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:86:12)
    at node:internal/main/run_main_module:23:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Copy link
Collaborator

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

I checked out the branch and ran it and tested the example "app".
I reviewed the code.
I was disappointed that the simple 'rest' interface, which I guess is gone/deprecated, had to be replaced by http with string keys.

import { Options } from "./codec/types";
import type { Document as BsonDocument } from "bson";
// BSON uses top level awaits, so use require for now
const bsonSerialize = require('bson').serialize;
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙄 THANKS BSON

@wilwade wilwade merged commit 2435994 into main May 14, 2024
1 check passed
@wilwade wilwade deleted the chore/package-updates branch May 14, 2024 19:01
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.

3 participants