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

Json: schema validation support fromJson for imported structs #3792

Closed
hasanaburayyan opened this issue Aug 11, 2023 · 2 comments · Fixed by #3944
Closed

Json: schema validation support fromJson for imported structs #3792

hasanaburayyan opened this issue Aug 11, 2023 · 2 comments · Fixed by #3944
Assignees
Labels
🐛 bug Something isn't working 🛠️ compiler Compiler

Comments

@hasanaburayyan
Copy link
Contributor

hasanaburayyan commented Aug 11, 2023

with the initial support for fromJson (#3648) methods on structs there is an issue when trying to call fromJson on structs that are defined within names spaces and in separate wing files

example:

file_1.w

struct Person {
	name: str;
}

file_2.w

bring "./file1.w" as f1;

let j = { name: "cool" };
f1.Person.fromJson(j);
@monadabot monadabot added this to Wing Aug 11, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New - not properly defined in Wing Aug 11, 2023
mergify bot pushed a commit that referenced this issue Aug 12, 2023
The changes in this PR make it possible to call `fromJson` on any compatible struct definition. 

## Implementation notes:
Previously we treated the jsification of structs as a no-op since there is not a JS equivalent. So with this change structs now JSify into a `Struct` file that contains a class with a static `jsonSchema` and a `fromJson` function which will allow for field validation at runtime. 

The schema generated adheres to: https://json-schema.org/understanding-json-schema/

take this simple example Wing code:
```js
struct MyStruct {
	myField: str;
	myOtherField: num;
}
```

this will now generate a JS file named `MyStruct.Struct.js` which looks like this:
```js
module.exports = function(stdStruct, fromInline) {
  class MyStruct {
    static jsonSchema() {
      return {
        id: "/MyStruct",
        type: "object",
        properties: {
          myField: { type: "string" },
          myOtherField: { type: "number" },
        },
        required: [
          "myField",
          "myOtherField",
        ],
        $defs: {
        }
      }
    }
    static fromJson(obj) {
      return stdStruct._validate(obj, this.jsonSchema())
    }
    static _toInflightType(context) {
      return fromInline(`require("./MyStruct.Struct.js")(${ context._lift(stdStruct) })`);
    }
  }
  return MyStruct;
};

```

The piece that brings this all together is the addition of the `Struct` class in our std that only has a `fromJson()` methods at the moment that is a Wing macro. The macro just calls the `fromJson()` method in the generated javascript.

### Misc
We want to stop the user at compile time from calling `fromJson` on a struct that cannot be represented by a Json value ie
```js
struct MyStruct {
	b: cloud.Bucket;
}
let j = {};
MyStruct.fromJson(j);
```

to prevent this I added a check in the typechecker for structs to confirm that if `fromJson` is called that all the fields in the struct are valid for conversion attempt

See image below for error when attempting:
<img width="664" alt="image" src="https://github.com/winglang/wing/assets/45375125/785a2fa6-8823-4fa2-aaa5-4bc8f7ed597f">


Closes: #3653
Closes: #3139

## TODO:
- [x] separate the work done here and the remaining work into different tickets.
- [x] update language reference

## Followup issues that are out of scope for this PR:
- #3792
- #3790
- #3789
- #3788

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@staycoolcall911 staycoolcall911 moved this from 🆕 New - not properly defined to 🤝 Backlog - handoff to owners in Wing Aug 13, 2023
@staycoolcall911 staycoolcall911 added 🐛 bug Something isn't working 🛠️ compiler Compiler labels Aug 13, 2023
@staycoolcall911
Copy link
Contributor

Upgraded to p1 based on feedback from @hasanaburayyan

@hasanaburayyan hasanaburayyan changed the title Json: schema validation support fromJson for structs in namespaces Json: schema validation support fromJson for imported structs Aug 23, 2023
@staycoolcall911 staycoolcall911 moved this from 🤝 Backlog - handoff to owners to 🏗 In progress in Wing Aug 24, 2023
@mergify mergify bot closed this as completed in #3944 Aug 24, 2023
mergify bot pushed a commit that referenced this issue Aug 24, 2023
Closes: #3792

Now supports calling fromJson on structs that were brought from other wing files

example: 
file_1.w
```js
struct Person {
	name: str;
}
```

file_2.w
```js
bring "./file1.w" as f1;

let j = { name: "cool" };
f1.Person.fromJson(j);
```

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Wing Aug 24, 2023
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.25.45.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🛠️ compiler Compiler
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants