-
Notifications
You must be signed in to change notification settings - Fork 27
TypeSpec SourceFile and Namespace components #324
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
base: feature/typespec
Are you sure you want to change the base?
TypeSpec SourceFile and Namespace components #324
Conversation
| case "model-property": | ||
| case "model": | ||
| case "union": | ||
| const invalidNameRegex = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we export printIdentifier from the compiler which will auto add the quotes when needed.
Or if we don't want to take dependency on the compiler here(maybe), could copy that function
| <SourceFile path="main.tsp" namespace={parentNamespace} /> | ||
| </Output> | ||
| ).toRenderTo({ | ||
| "main.tsp": `namespace My.Namespace;\n\n\n`, // why do we need to do this for this assertion to pass? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you can probably use d` like in the other tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need d with toRendeRTo it will already dedent automatically
| path: string; | ||
|
|
||
| /** If present, it defines a file-level namespace (if not present, it uses the global namespace) */ | ||
| namespace?: NamespaceSymbol; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably we could receive a symbol or string here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could, but I thought receiving the prop already typed was better. Although absolutely open to change the approach if it makes more sense.
| export function SourceFile(props: SourceFileProps) { | ||
| const sourceFileScope = new SourceFileScope(props.path); | ||
|
|
||
| const globalNs = getGlobalNamespace(useBinder()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this we need to look at, in tsp the global namespace still belongs to a file (what really matters for symbol resolution are files). So here we should "create" a global namespace scope with file as the parent.
| * containers like namespaces. This scope is a member scope whose | ||
| * member symbol is a NamedTypeSymbol. | ||
| */ | ||
| export class TypeSpecNamedTypeScope extends TypeSpecScope { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this in this PR?
| // e.g. if refkey is for bar in enum type foo, and | ||
| // foo is in the same namespace as the refkey, then | ||
| // the result would be foo.bar. | ||
| export function ref( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this for this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a cascading dependency necessary to define Reference.tsx so we can pass that to CoreSourceFile : https://github.com/alloy-framework/alloy/pull/324/files#diff-668821ea9c50226ee835e35b54cddd4afc9313f85249093722a652ad3d763187R51
commit: |
Open questions:
test/components/namespace.test.tsxwe are targeting to make that unit test pass but nothing renders. We are struggling to find the minimal code necessary for the setup to work (the code has been mostly lifted from the csharp package).