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

chore: Update @hapi/joi #25797

Closed
wants to merge 18 commits into from
Closed

Conversation

herecydev
Copy link
Contributor

Fixes #25764

@herecydev herecydev requested review from a team as code owners July 16, 2020 16:31
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 16, 2020
@herecydev herecydev changed the title Update packages Update @hapi/joi Jul 16, 2020
@herecydev herecydev changed the title Update @hapi/joi WIP: Update @hapi/joi Jul 17, 2020
@pieh pieh added type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jul 17, 2020
@hueniverse
Copy link

Please switch the dependency to joi (no longer under @hapi/joi). If I can help in any way, please let me know. Thanks!

@hueniverse
Copy link

Are you back on the hapi version because of typescript?

@herecydev
Copy link
Contributor Author

Are you back on the hapi version because of typescript?

Yep, was going to shoot a message across but you beat me to it! If there's an easy way to solve this let me know! Otherwise I feel like taking a phased approach works best for me:

  1. Get stuff upgraded so it's not deprecated anymore
  2. Change to the joi package (as I understand it's the same thing but it's future will be in the sideway repo?)

@hueniverse
Copy link

I will get the type definitions copied over to the original namespace so it will resolve this. Meanwhile your plan sounds good.

@herecydev
Copy link
Contributor Author

Eran, I'm struggling to understand the translation of joi-to-graphql.js. There's a lot of _meta and _children that I'm unsure of. I got somewhere by using .describe() but I'm new to joi so probably need some advice here.

Hope that's okay?

@hueniverse
Copy link

I'm surprised how deep this code goes into joi internals...

You are on the right track with describe(). It should includes everything you need since that information is enough to rebuild the schema. Take a look at https://github.com/sideway/joi/blob/master/test/manifest.js#L20 for an example.

I am happy to try and help but I can't figure out how to run the tests for this code.

@herecydev
Copy link
Contributor Author

herecydev commented Jul 20, 2020

So a little further on... It looks like the file enumerates the keys of the current schema and switches based on whether it's an object, array or lazy. The fields returned looked like:

[
      {
        key: 'name',
        schema: {
          isJoi: true,
          _type: 'string',
          _settings: null,
          _valids: [InternalSet],
          _invalids: [InternalSet],
          _tests: [],
          _refs: [],
          _flags: {},
          _description: null,
          _unit: null,
          _notes: [],
          _tags: [],
          _examples: [],
          _meta: [],
          _inner: {},
          _currentJoi: [Object]
        }
      },
      {
        key: 'description',
        schema: {
          isJoi: true,
          _currentJoi: [Object],
          _type: 'string',
          _settings: null,
          _baseType: undefined,
          _valids: [InternalSet],
          _invalids: [InternalSet],
          _tests: [],
          _refs: [],
          _flags: [Object],
          _description: null,
          _unit: null,
          _notes: [],
          _tags: [],
          _examples: [],
          _meta: [],
          _inner: {}
        }
      },
...
]

I got something similar using object.extract() which gave me the matching fields schema. I just don't really know how to procede with schema._tests or schema._flags.presence. I can't find any docs on those fields

For reference I've never used Joi so this is a bit of a learning curve for me. @ascorbic

@hueniverse
Copy link

@herecydev You should only use describe(). Everything else is joi internals and will break randomly as I change things.

@herecydev
Copy link
Contributor Author

@ascorbic @mxstbr Are you able to provide some assistance with this file? There's too much internal behavior going on but it's really difficult to know the solution without some deeper knowledge in this area

@mxstbr
Copy link
Contributor

mxstbr commented Jul 23, 2020

We copied-and-edited that code from https://github.com/xogroup/joi2gql, so we're not familiar with it either unfortunately 😬

@herecydev
Copy link
Contributor Author

😬 that's not ideal. I've raised a ticket upstream.

@herecydev
Copy link
Contributor Author

@mxstbr not sure how to move this forward. https://github.com/xogroup/joi2gql looks very unloved.

@herecydev herecydev marked this pull request as draft August 5, 2020 18:44
@hueniverse
Copy link

Note that joi v17.2.0 is published with TS support under joi.

@sidharthachatterjee sidharthachatterjee changed the title WIP: Update @hapi/joi chore: Update @hapi/joi Aug 12, 2020
@sidharthachatterjee sidharthachatterjee marked this pull request as ready for review August 12, 2020 08:16
@LekoArts
Copy link
Contributor

LekoArts commented Feb 1, 2022

I think this PR got quite stale and diverged from the current master. We still want to do it, but a fresh start should be better than dealing with all the merge conflicts!

We removed gatsby-recipes so the difficult parts should be gone

@LekoArts LekoArts closed this Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gatsby using deprecated @hapi/joi
6 participants