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 #62

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bruderstein
Copy link
Contributor

Update Hapi (to latest), TypeScript (to 4.x) and jest (to latest)

Fixed the import parser to match multiple import statements (useful if split between import and import type), and to also match import type ... statements.

Dropped support for node 14 with Hapi 21.

Will do TypeScript 5.x separately.

Dave Brotherstone added 3 commits December 4, 2023 09:27
Major bump for Hapi was basically just the CORS OPTIONS response change
from 204 -> 200.

TypeScript was just a couple of minor type fixes, mostly for updated
types for node.

BREAKING CHANGE: drop support for node 14.
We now need to support `import type` and multiple import lines, such
such that there can be an `import type { ...} from '...intervene'` and
a second `import { ... } from 'intervene'` line. This also meant we
could no longer rely on importing `ProxyConfig` as the confirmation
that this was indeed an intervene import line. So we now rely on there
only being intervene known imports in the statement _and_ that the word
`intervene` is somewhere in the import path.

Note that this is related to the TypeScript update, as TS4 introduces
`import type`.
Since the name change, we needed to update the hostname to the new
vercel.app hostname for the tests, from
github.com/bruderstein/intervene-test.

Note that this still doesn't work against production, as the
intervene-test project needs updating to run with the new vercel
environment (was previous now.sh, which was renamed).

But running the tests against that repo running locally works now
Test on 16,18,20
@bruderstein
Copy link
Contributor Author

Tagging @pablofmena @lukehorvat :)

Copy link
Contributor

@pablofmena pablofmena left a comment

Choose a reason for hiding this comment

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

lvgtm1 sorry for the late reply

@bruderstein Christmas visit ❤️

@@ -1,4 +1,4 @@
const REAL_EXTERNAL_HOST = 'https://intervene-test.bruderstein.now.sh';
const REAL_EXTERNAL_HOST = 'https://intervene-test.bruderstein.vercel.app';
Copy link
Contributor

Choose a reason for hiding this comment

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

what's served from here? are you planning on hosting it forever? 😅

@@ -9,7 +9,7 @@
"license": "MIT",
"scripts": {
"prepare": "npm run build",
"build": "tsc -p . -d",
"build": "tsc -p . -d && cp src/brotli-decode.* dist/src",
Copy link
Contributor

Choose a reason for hiding this comment

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

does this serve as a replacement for https://github.com/soundcloud/intervene/blob/master/Makefile#L8? should we favor running the npm script directly over the make target?

Copy link
Contributor

@lukehorvat lukehorvat left a comment

Choose a reason for hiding this comment

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

nice one, thanks dave!

@@ -17,15 +17,16 @@ export interface CertificateOptions {
const HOUR_IN_MS = 60 * 60 * 1000;
const DAY_IN_MS = 24 * HOUR_IN_MS;

const certPromises: { [key: string]: Promise<tls.SecureContextOptions> } = {};
const certPromises: { [key: string]: Promise<tls.SecureContextOptions> | undefined } = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Suggested change
const certPromises: { [key: string]: Promise<tls.SecureContextOptions> | undefined } = {};
const certPromises: { [key: string]?: Promise<tls.SecureContextOptions> } = {};

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

Successfully merging this pull request may close these issues.

None yet

3 participants