-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Upgrade hapi to @hapi/hapi 18.4.1 #61959
Conversation
Pinging @elastic/kibana-app-arch (Team:AppArch) |
2213ab2
to
c67ae3e
Compare
The `ApplicationState` interface has in the new version of `@types/hapi__hapi` been split up into: - `RequestApplicationState` - `ResponeApplicationState` - `ServerApplicationState` See PR for more info: DefinitelyTyped/DefinitelyTyped#36361 It looks like the one we want to use is `RequestApplicationState`.
Without this change we would get the following TypeScript error: This condition will always return 'false' since the types 'AuthSettings' and 'boolean' have no overlap. According to the types, `authOptions` can either be an object, `undefined` (bot of which we already check for), or the litteral value `false`. Previously the types were a bit more loose and also allow a string since it was re-using the `RouteOptions` interface (which does allow you to specify the `auth` as a string). When reading it back from `request.route.settings.auth` it doesn't appear to be possible for the value to be a string however.
A lot of the legacy plugin code needs updating to support this new version of hapi, so I've put this on the back burner for now, while I'm waiting for more plugins to be migrated to the New Platform. This will make the migration to hapi 18 much easier. However, hapi 18 is a pre-requisite for upgrading to Node.js 12 as it seems like that version 17 isn't compatible with Node.js 11+, even though it's stated on the hapi website. If this is true, we need to push on with this despite all the legacy plugins (upstream issue: hapijs/hapi#3898, fix: hapijs/hapi@a3a5ca9). |
@watson Hey Thomas, are you planning to pick this back up anytime soon? |
@joshdover yeah I'm working on it together with the Node.js upgrade currently. However I'll most likely do another PR as this has gotten pretty stale... I'll update this PR with more information when I'm there |
Closing in favour of #80468 |
💔 Build Failed
Failed CI Steps
History
To update your PR or re-run it, just comment with: |
Replaced by #80468
List of upgraded modules:
hapi
@hapi/hapi
boom
@hapi/boom
hapi-auth-cookie
@hapi/cookie
@elastic/good
@hapi/good
8.1.1-kibana28.2.4Minor 💛h2o2
@hapi/h2o2
hoek
@hapi/hoek
inert
@hapi/inert
podium
@hapi/podium
vision
@hapi/vision
Blocked by