Skip to content

WIP: graplix v2 #9

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

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

WIP: graplix v2 #9

wants to merge 8 commits into from

Conversation

ho991217
Copy link
Collaborator

Update to Graplix V2

This PR includes updates to v2.

🔥 New features

  • Create schema and identifier with OpenFGA DSL
  • Automatic Type Generation
  • Implicit identify with id property

📝 Checklist

  • validator
  • typegen
  • resolver integration with graplix

Copy link
Member

@tonyfromundefined tonyfromundefined left a comment

Choose a reason for hiding this comment

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

수고하셨어요! 저는 아래와 같은 컨벤션을 사용하고 있어서 어떻게 생각하시는지 + 혹시 괜찮으시다면 반영이 가능할지 궁금해요.

"모든 export class, export function는 해당 class와 function 이름과 같은 파일 명을 갖는다."

  • as-is: errors.ts -> to-be: UnimplementedError.ts, MultipleError.ts와 같은식으로 분리
  • as-is: exceptions.ts -> to-be: ExceptionCollector.ts로 이름 변경

dsl 폴더 이름을 language라는 이름으로 바꾸면 더 좋을거같은데 어떠세요? dsl이라고 하는 폴더명이 맥락이 좀 있는거같아서 풀고 싶어서요!

Comment on lines 1 to 31
export abstract class SingleError extends Error {
constructor(public message: string) {
super(message);
}

toString() {
return this.message;
}
}

export class UnimplementedSingleError extends SingleError {
constructor(featureName: string) {
super(`${featureName} is not implemented`);
this.name = "UnimplementedError";
}
}

abstract class MultipleError<T = SingleError> extends Error {
constructor(public errors: Array<T>) {
super(
`${errors.length} error${errors.length > 1 ? "s" : ""} occurred:\n\t* ${errors.join("\n\t* ")}\n\n`,
);
this.errors = errors;
}

toString() {
return this.message;
}
}

export class UnimplementedError extends MultipleError<UnimplementedSingleError> {}
Copy link
Member

Choose a reason for hiding this comment

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

에러 네이밍이 조금 더 명시적이면 좋을거같아서 아래와 같이 수정해봤어요~

  • Single, Multiple 개념이 헷갈려서 이렇게 정리하면 좋을거같아요.
    • Single: 생략
    • Multiple: "Multiple" prefix 붙이기
  • toString() 구현이 들어간걸로 보이는데, 기본적으로 Error 객체에 구현되어있지 않나요? 따로 override하신 이유가 궁금해요.
Suggested change
export abstract class SingleError extends Error {
constructor(public message: string) {
super(message);
}
toString() {
return this.message;
}
}
export class UnimplementedSingleError extends SingleError {
constructor(featureName: string) {
super(`${featureName} is not implemented`);
this.name = "UnimplementedError";
}
}
abstract class MultipleError<T = SingleError> extends Error {
constructor(public errors: Array<T>) {
super(
`${errors.length} error${errors.length > 1 ? "s" : ""} occurred:\n\t* ${errors.join("\n\t* ")}\n\n`,
);
this.errors = errors;
}
toString() {
return this.message;
}
}
export class UnimplementedError extends MultipleError<UnimplementedSingleError> {}
enum ErrorCode {
UNIMPLEMENTED = "UNIMPLEMENTED",
}
export class UnimplementedError extends Error {
code = ErrorCode.UNIMPLEMENTED;
constructor(featureName: string) {
super(`${featureName} is not implemented`);
}
}
export class MultipleError<T extends Error> extends Error {
constructor(public errors: Array<T>) {
super(
`${errors.length} error${errors.length > 1 ? "s" : ""} occurred:\n\t* ${errors.join("\n\t* ")}\n\n`,
);
}
toString() {
return this.message;
}
}
export class MultipleUnimplementedError extends MultipleError<UnimplementedError> {}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • toString() 구현이 들어간걸로 보이는데, 기본적으로 Error 객체에 구현되어있지 않나요? 따로 override하신 이유가 궁금해요.

fga/language 쪽 error 구현체를 가져오다보니 그런 것 같아요. 그 외에 제안주신 내용은 반영되었어요! 4fe1ce1

src/dsl/fga.ts Outdated
@@ -0,0 +1,8 @@
import { transformer } from "@openfga/syntax-transformer";

export function fga([data]: TemplateStringsArray): ReturnType<
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export function fga([data]: TemplateStringsArray): ReturnType<
export function parse([data]: TemplateStringsArray): ReturnType<

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그래프큐엘 함수에서 착안했어요!

const query = graphql``;

아래 용도로 생각했을 때, parse라는 이름보다 맥락을 전달하는데에 효과적이라고 생각해요!

const scheme = fga`
  model
    schema 1.1

  type team
    relations
      define member: [user]
`;

Copy link
Member

Choose a reason for hiding this comment

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

좋네요! ㅎㅎ 엣지케이스들은 차차 쌓아나가면 좋을거같아요.

Comment on lines 6 to 8
raiseUnimplementedError(featureName: string) {
this.errors.push(new UnimplementedSingleError(featureName));
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
raiseUnimplementedError(featureName: string) {
this.errors.push(new UnimplementedSingleError(featureName));
}
captureUnimplementedError(featureName: string) {
this.errors.push(new UnimplementedSingleError(featureName));
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reflected at: 80e3610

Copy link

changeset-bot bot commented Mar 26, 2025

⚠️ No Changeset found

Latest commit: b35bfa1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

ho991217 and others added 6 commits March 26, 2025 10:12
Restructure the codebase to consolidate language-related exports and update import paths for better maintainability and clarity. This includes moving errors to the language module and adjusting imports accordingly.
Refactor error handling by introducing `UnimplementedError` and `MultipleError` classes to replace the previous `SingleError` and `UnimplementedSingleError`. Update validation logic to use the new error classes and rename `ExceptionCollector` to improve clarity. This change enhances maintainability and simplifies the error handling structure.
The `validate` function is now called after transforming the DSL to JSON object to ensure the model is valid before returning the AST. This improves the robustness of the FGA model processing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants