-
Notifications
You must be signed in to change notification settings - Fork 23
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
Generate Data Table #73
Conversation
- extend generator
This reverts commit 4fac1a9.
- formatting and localization
Issue 56
Detail - TextInput + generator
…to Generate-list
Generate-list
@@ -9,4 +9,6 @@ const components = { | |||
footer: defineComponent('TableFooter', 'grommet') | |||
} | |||
|
|||
export default components | |||
export const GrommetDtTableComponents = { |
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.
Let's use Data
instead of Dt
(I mean GrommetDataTableComponents
)
return uniqueImportDeclarations; | ||
} | ||
|
||
createVariableStatement(variableName:string, expression:ts.Expression, flag: ts.NodeFlags): ts.VariableStatement { |
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.
too generic file name
I propose following file name: variables/factory.ts
Simmilar to what we have in 3a7d7b4#diff-3f1683d9657cf7914e8dd0639b35f4b5c0c14d48d5054aeae1637f632bdeebac
) | ||
} | ||
|
||
createNameSpaceImport(namespace: string, module: string): ts.ImportDeclaration { |
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.
too generic file name
I propose following file name: imports/factory.ts
Simmilar to what we have in 3a7d7b4#diff-3f1683d9657cf7914e8dd0639b35f4b5c0c14d48d5054aeae1637f632bdeebac
|
||
class TypescriptHelper { | ||
|
||
createImportDeclaration(importSpecifier: string, module:string): ts.ImportDeclaration { |
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.
too generic file name
I propose following file name: imports/factory.ts
Simmilar to what we have in 3a7d7b4#diff-3f1683d9657cf7914e8dd0639b35f4b5c0c14d48d5054aeae1637f632bdeebac
}); | ||
|
||
//group it by module specifier | ||
let grouppedImports = new Map(); |
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.
please add types to Map key and value
uniqueImports(imports:ts.ImportDeclaration[]): ts.ImportDeclaration[]{ | ||
let uniqueImports: Map<string,string> = new Map<string, string>(); | ||
|
||
//get unique imports |
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.
please extract implementation of this function into multiple functions in imports/unique.ts
each of these comments inside this function should be an function
} | ||
|
||
//TODO: improve this to be able also process namespace imports | ||
uniqueImports(imports:ts.ImportDeclaration[]): ts.ImportDeclaration[]{ |
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.
move this function please in new file: imports/unique.ts
Simmilar to what we have in 3a7d7b4#diff-02b50984ce1bcacda4cdc94fdf741c13689ed25757b4e6415bab8b6f83c56432
import { TableType, UiFramework } from '../definition/context-types' | ||
import { Entity } from "./entity" | ||
|
||
export default interface GenerationContext { |
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.
what about CodeGenContex
I think the CodeGen
is well known in IT community
useFormatter: boolean | ||
tableType: TableType | ||
uiFramework: UiFramework | ||
entity: Entity |
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 should rename property name entity
to express the semantics/intention. I propose inputEntity: Entity
|
||
export default interface GenerationContext { | ||
useFormatter: boolean | ||
tableType: TableType |
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 am thinking about documenting reasonable deafaults. We might want to specify Options data type and an instance with default values.
@@ -9,4 +9,5 @@ export interface Entity { | |||
export interface Property { | |||
getName(): string | |||
getType(): Type<ts.Type> | |||
getTypeText(): string |
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.
What is the purpose of this getter? Isn't it a hack relasted to typescript AST?
import ts, { factory } from "typescript" | ||
import { DetailComponentDefinitionBase } from '../../../definition/detail-definition-core' | ||
import { Component } from '../../react-components/react-component-helper' | ||
import { camalizeString } from '../../../utils/utils' |
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.
Consider package strings/case.ts
this.intlFormatter = new ReactIntlFormatter(this.context, this._imports); | ||
} | ||
|
||
getProperties(): Property[]{ |
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.
please format the code properly (I mean the space before {
)
return camalizeString(this.context.entity.getName()) | ||
} | ||
|
||
protected getRowsIdentifier() : ts.Identifier { |
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.
what about more specific name: RowsVariable
to express the semantins (we allready have the data type - we do not neet to repeat it in the name). Same comment for next method...
} | ||
|
||
generateTableComponent(): PageComponent { | ||
var statements = this. createStatements() |
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.
extra white space...
} | ||
|
||
private createHeader(): ts.JsxChild { | ||
const headerComponent = this.prepareComponent(this.getTableDefinition().header); |
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.
please assign result of the this.getTableDefinition()
to a variable
[ | ||
factory.createPropertyAssignment( | ||
factory.createIdentifier("height"), | ||
factory.createNumericLiteral("400") |
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.
should not be hardcoded
I propose a reasonable default in context.options.dataTable.height
...
true | ||
) | ||
], | ||
factory.createJsxClosingElement(factory.createIdentifier("div")) |
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.
Please call funtion that wraps opening and closing element 745110c#diff-0c7eb128f653257f8272e20d2f7a0b6c5f1f07768644ee93f808be3383af5da8L43
if(this._context.tableType === TableType.DataTable) { | ||
switch(this._context.uiFramework){ | ||
case UiFramework.Mui: | ||
generator = new MuiDataTableGenerator(this._context); |
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.
Just return (no local variable and break keyword necessary)
return factory.createJsxElement(jsxOpeningElement, children ?? [], jsxClosingElement) | ||
} | ||
|
||
export function createJsxAttribute(attribute:string, attributeValue:ts.Identifier|string) { |
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.
packages/react-lowcode/src/attributes/factory.ts ...
the same comment for all functions in this file
importDeclaration: ts.ImportDeclaration | ||
} | ||
|
||
export interface PageComponent { |
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 would like to understand the purpose of the PageComponent
during the code generation
} | ||
|
||
//TODO: add hook support | ||
export default class ReactIntlImperative { |
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.
naming: what about intl hook instead intl imperative?
import { Entity } from "./entity" | ||
|
||
export default interface GenerationContext { | ||
useFormatter: boolean |
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.
please use enum also for formatter
Merge commits from iteria-app/lowcode into mat-app/lowcode
- create simple entry point for generation
|
||
class codegen { | ||
|
||
generate(): string { |
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.
export function generate()
would be more appropriate
Extend generator to be able to generate basic table and advanced data tables from material-ui and grommet frameworks.