feat: use @stoplight/json-schema-tree #91

P0lip · 2020-11-14T01:49:57Z

There is one major breaking change from component's perspective - $refs are resolved eagerly by default and there is no way to alter this behavior.
We had that option enabled at all times in all projects anyway, thus it only added redundant complexity for us.
That said, a few tests had to be updated to account for that change.
There was a number of tests updated as a result of better handling of a particular scenario.

$refs

Before and after

3.0

This PR

3.0

This PR

3.0*

This PR

3.0

This PR

The array flattening logic adds a tiny bit of complexity, but I decided to implement it so that we can leverage this updated component in existing products. I'm aware that it's likely to be thrown away in the next iteration of JSV, but until then, we couldn't benefit from the improvements.

I decided to tweak the naming as well - hopefully I did a good job here 😆

Changelog

Full list of notable differences vs existing 3.0 (certain pieces were extracted to separate PRs, but listing them here to keep track of them):

enforced $ref resolving
improved flattening - reduced nesting in some case (see array + combiner scenario I attached)
better oneOf/anyOf merging
alert icon for $refs errors (see screenshot)
more correct properties count (see the User in above screenshot, I decided to display the number of actual rows) + more cases covered
top bar is gone chore: remove top bar #96 - never used, Next heritage
better usage of titles (test: reorganize shared components tests #97 + this PR - see Property.spec.tsx)
format is displayed even for type-less nodes, i.e. { format: 'date-time' }
ordered array items are handled more properly ({ "type": "array", "items": [item1, item2, ...] })
Studio bonus -> JST emits error event when all of merging fails making https://github.com/stoplightio/platform-internal/issues/4902 a piece of cake

src/components/JsonSchemaViewer.tsx

src/tree/utils/hasComplexChild.ts

P0lip · 2020-12-17T02:11:23Z

@marbemac feel free to take a look at the changelog to see if there isn't anything you would like me to revert.

P0lip · 2020-12-17T02:19:16Z

All tests are passing now.

P0lip · 2020-12-17T02:20:17Z

🙄

edit:
ah nvm, the package was private

src/__tests__/index.spec.tsx

P0lip · 2020-12-17T22:44:08Z

an additional question - shall we make json-schema-tree repo public?

marbemac · 2020-12-18T00:08:48Z

an additional question - shall we make json-schema-tree repo public?

Yeah that's fine. When we're ready to merge all of this we should also release 1.0 of json-schema-tree so that it's not in forever beta hell :).

There is one major breaking change from component's perspective - $refs are resolved eagerly by default and there is no way to alter this behavior.

What do you mean by eagerly? All the way down or only refs that are present at the depth that is expanded already? e.g. if really deep schema, and everything is collapsed to start, will deep $refs be eagerly resolved or only resolved on demand as user expands the tree?

Also are local $ref pointers managed out of the box?

P0lip · 2020-12-18T10:54:42Z

What do you mean by eagerly? All the way down or only refs that are present at the depth that is expanded already? e.g. if really deep schema, and everything is collapsed to start, will deep $refs be eagerly resolved or only resolved on demand as user expands the tree?

Only $refs refs that are present at the depth that is expanded already.

Current 3.0 had a possibility to avoid upfront dereferencing entirely and do it when user expands the ref.
This is actually the legacy behavior we had prior to shouldResolveEagerly option, where all $refs were rendered in the schema and you could expand them.
Does that make sense?
As of this PR, we only show $refs that couldn't be resolved for some reason and display that warning icon.

Also are local $ref pointers managed out of the box?

Yup, they are.

P0lip · 2020-12-18T14:31:16Z

I'll try to extract a few changes to separate PRs to make reviewing more feasible.

package.json

marcelltoth · 2020-12-21T11:40:54Z

package.json

    "@stoplight/react-error-boundary": "^1.0.0",
    "@stoplight/tree-list": "^5.0.3",
-    "classnames": "^2.2.6",
-    "lodash": "^4.17.15"


What, no lodash? I can't believe that, how do you even write code? 😇

IIRC lodash was used in 2-3 spots in JSV. In fact, the only useful fn was pick, but I got rid of it in JST completely.

I was just kidding 😄 It's nice that you got rid of it. It's still a transient dep - and likely will be for the foreseeable future - but less direct dependencies are always nice.

Yeah, I'm not a fan of bringing the whole library if you only use it once or twice. 😆
It's totally okay if you actually use it, but makes little to no sense if you only use pick in a single spot.

True, especially because it doesn't tree shake as well & as easily as it should. At least it didn't about a year ago, as it's mainly CJS

billiegoose

The truth is, this PR changes too much code for to effectively review. I'm gonna allow it because I don't see any other choice, but I think in the future, we should try to plan this kind of thing better.

It's good changes though. 👍🏻

Basically, my attitude is: we'll play around with it and try to break it when you make the PR to platform-internal. But you say it works good when yalced so 🤷🏻‍♂️ I believe ya.

P0lip · 2020-12-23T20:58:24Z

Frankly, this PR is smaller than it may seem.
The diff is a bit misleading here, since I added a bunch of tests. Most of them have been already extracted to separate PRs, but some were added yesterday.

git diff --oneline --stat origin/beta -- src/ ':!src/__fixtures__' ':!src/__stories__' ':!**/__tests__/**' 

 src/components/JsonSchemaViewer.tsx   |  53 ++---
 src/components/SchemaRow.tsx          | 170 +++++++-------
 src/components/SchemaTree.tsx         |   6 +-
 src/components/shared/Divider.tsx     |  14 +-
 src/components/shared/Format.tsx      |  30 ++-
 src/components/shared/Name.tsx        |  13 ++
 src/components/shared/Property.tsx    | 131 +++--------
 src/components/shared/Types.tsx       | 140 ++++--------
 src/components/shared/Validations.tsx |  13 +-
 src/components/shared/index.ts        |   1 +
 src/consts.ts                         |  27 +++
 src/contexts/SchemaNode.ts            |   4 +
 src/contexts/SchemaTree.ts            |  12 +
 src/contexts/TreeListNode.ts          |   6 +
 src/contexts/ViewModeProvider.ts      |   5 +
 src/contexts/index.ts                 |   4 +
 src/errors.ts                         |   3 -
 src/guards/isCombiner.ts              |   5 +
 src/guards/isNonNullable.ts           |   3 +
 src/hooks/index.ts                    |   4 +-
 src/hooks/useMetadata.ts              |   8 -
 src/hooks/useSchemaNode.ts            |   8 +
 src/hooks/useSchemaTree.ts            |   7 +
 src/hooks/useTreeListNode.ts          |   7 +
 src/index.ts                          |   5 +-
 src/tree/index.ts                     |   4 +-
 src/tree/metadata.ts                  |  39 ----
 src/tree/tree.ts                      | 402 +++++++++++++++++++++++-----------
 src/tree/types.ts                     |  29 +++
 src/tree/utils/canStepIn.ts           |  14 --
 src/tree/utils/createErrorTreeNode.ts |  22 --
 src/tree/utils/mergeAllOf.ts          |  64 ------
 src/tree/utils/mergeOneOrAnyOf.ts     |  45 ----
 src/tree/utils/populateTree.ts        | 294 -------------------------
 src/tree/utils/walk.ts                | 117 ----------
 src/types.ts                          |  58 +----
 src/utils/flattenTypes.ts             |  55 -----
 src/utils/generateId.ts               |   1 -
 src/utils/getAnnotations.ts           |  10 -
 src/utils/getCombiners.ts             |  25 ---
 src/utils/getMetadata.ts              |  10 -
 src/utils/getPrimaryType.ts           |  22 --
 src/utils/getValidations.ts           |  56 -----
 src/utils/guards.ts                   |  19 --
 src/utils/index.ts                    |   2 +
 src/utils/inferType.ts                |  20 --
 src/utils/isCombiner.ts               |   5 -
 src/utils/isPropertyRequired.ts       |  10 +
 src/utils/isSchemaViewerEmpty.ts      |  14 --
 src/utils/isValidType.ts              |   6 -
 src/utils/normalizeRequired.ts        |   6 -
 src/utils/printName.ts                |  65 ++++++
 52 files changed, 710 insertions(+), 1383 deletions(-)

stoplight-bot · 2020-12-23T21:04:10Z

🎉 This PR is included in version 4.0.0-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

BREAKING CHANGE: shouldResolveEagerly prop has been removed

marcelltoth · 2020-12-28T09:56:00Z

I'd still argue that a +710-1383 PR is effectively impossible to review thoroughly, but I'm glad it's in 😄

marcelltoth · 2020-12-28T10:02:41Z

Release failed unfortunately :\

P0lip added the WIP Work in progress label Nov 14, 2020

P0lip self-assigned this Nov 14, 2020

P0lip force-pushed the feat/use-schema-tree branch 2 times, most recently from 463e33f to 4cafc50 Compare November 17, 2020 14:46

This was referenced Dec 10, 2020

chore: setup ESLint and bump deps #95

Merged

test: reorganize shared components tests #97

Merged

refactor: move Format component out of Validations #98

Merged

P0lip added the enhancement New feature or request label Dec 16, 2020

P0lip force-pushed the feat/use-schema-tree branch from cbb0514 to 7d40348 Compare December 16, 2020 12:05

P0lip commented Dec 16, 2020

View reviewed changes

src/components/JsonSchemaViewer.tsx Show resolved Hide resolved

P0lip commented Dec 16, 2020

View reviewed changes

src/tree/utils/hasComplexChild.ts Outdated Show resolved Hide resolved

P0lip marked this pull request as ready for review December 17, 2020 02:19

P0lip requested a review from a team December 17, 2020 02:19

P0lip force-pushed the feat/use-schema-tree branch from b54ea93 to 7e13e2c Compare December 17, 2020 02:26

P0lip removed the WIP Work in progress label Dec 17, 2020

P0lip requested a review from marbemac December 17, 2020 02:28

P0lip commented Dec 17, 2020

View reviewed changes

src/__tests__/index.spec.tsx Outdated Show resolved Hide resolved

P0lip mentioned this pull request Dec 18, 2020

test: more output tests #100

Merged

P0lip force-pushed the feat/use-schema-tree branch from 0230299 to 4cc5854 Compare December 18, 2020 17:49

philsturgeon mentioned this pull request Dec 18, 2020

JSV: Show oneOf/anyOf as a dropdown stoplightio/elements#667

Closed

marbemac reviewed Dec 18, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

marcelltoth reviewed Dec 21, 2020

View reviewed changes

P0lip force-pushed the feat/use-schema-tree branch from 4cc5854 to bdc4225 Compare December 22, 2020 14:37

billiegoose approved these changes Dec 23, 2020

View reviewed changes

feat: integrate json-schema-tree

b182faa

P0lip force-pushed the feat/use-schema-tree branch from bdc4225 to b182faa Compare December 23, 2020 20:51

P0lip merged commit 775ad72 into beta Dec 23, 2020

P0lip deleted the feat/use-schema-tree branch December 23, 2020 21:00

stoplight-bot added the released on @beta label Dec 23, 2020

P0lip added a commit that referenced this pull request Dec 23, 2020

feat: integrate @stoplight/json-schema-tree (#91)

0635f1f

BREAKING CHANGE: shouldResolveEagerly prop has been removed

marcelltoth mentioned this pull request Dec 28, 2020

Release failed #102

Closed

P0lip mentioned this pull request Mar 9, 2021

refactor: simplify JSV #109

Merged

philsturgeon mentioned this pull request Jun 21, 2021

Release v4.0.0 stable #152

Closed

This was referenced Nov 30, 2021

feat: tree-ui and path crumbs #179

Merged

chore: remove html output tests #180

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use @stoplight/json-schema-tree #91

feat: use @stoplight/json-schema-tree #91

P0lip commented Nov 14, 2020 •

edited

P0lip commented Dec 17, 2020

P0lip commented Dec 17, 2020

P0lip commented Dec 17, 2020 •

edited

P0lip commented Dec 17, 2020

marbemac commented Dec 18, 2020

P0lip commented Dec 18, 2020 •

edited

P0lip commented Dec 18, 2020

marcelltoth Dec 21, 2020

P0lip Dec 21, 2020

marcelltoth Dec 21, 2020

P0lip Dec 21, 2020

marcelltoth Dec 21, 2020

billiegoose left a comment •

edited

P0lip commented Dec 23, 2020

stoplight-bot commented Dec 23, 2020

marcelltoth commented Dec 28, 2020

marcelltoth commented Dec 28, 2020

feat: use @stoplight/json-schema-tree #91

feat: use @stoplight/json-schema-tree #91

Conversation

P0lip commented Nov 14, 2020 • edited

$refs

Before and after

Changelog

P0lip commented Dec 17, 2020

P0lip commented Dec 17, 2020

P0lip commented Dec 17, 2020 • edited

P0lip commented Dec 17, 2020

marbemac commented Dec 18, 2020

P0lip commented Dec 18, 2020 • edited

P0lip commented Dec 18, 2020

marcelltoth Dec 21, 2020

Choose a reason for hiding this comment

P0lip Dec 21, 2020

Choose a reason for hiding this comment

marcelltoth Dec 21, 2020

Choose a reason for hiding this comment

P0lip Dec 21, 2020

Choose a reason for hiding this comment

marcelltoth Dec 21, 2020

Choose a reason for hiding this comment

billiegoose left a comment • edited

Choose a reason for hiding this comment

P0lip commented Dec 23, 2020

stoplight-bot commented Dec 23, 2020

marcelltoth commented Dec 28, 2020

marcelltoth commented Dec 28, 2020

P0lip commented Nov 14, 2020 •

edited

P0lip commented Dec 17, 2020 •

edited

P0lip commented Dec 18, 2020 •

edited

billiegoose left a comment •

edited