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

feat(CodeEditor): Changing schemas based on the content #172

Merged
merged 5 commits into from
Oct 4, 2023

Conversation

mmelko
Copy link
Contributor

@mmelko mmelko commented Oct 2, 2023

This PR introduces EntitySchemaConfig which is meant to be to replace SchemasStore. Schemas should be possible to configure within the entities-context. Also this PR introduces currentEntityfield which is used for holding the current EntityType. After change of the currentEntity proper schema should be also reloaded in the CodeEditor.

related to #136 #135

@mmelko mmelko requested a review from a team October 2, 2023 09:14
@mmelko mmelko force-pushed the schema-config branch 2 times, most recently from 666a5ae to 22d68a7 Compare October 3, 2023 12:43
@mmelko
Copy link
Contributor Author

mmelko commented Oct 3, 2023

I updated the PR, incorporated your suggestion and:

  • new enum SourceSchemaType .. as we discussed
  • renamed currentEntity to currentSchemaType
  • renamed EntitySchemaConfig to SourceSchemaConfig
  • add isKamelet and isIntegration with stubs and tests to being able to assign the schema properly

If the names are off .. I can change them :)

@@ -14,25 +16,30 @@ interface SourceCodeProps {

export const SourceCode: FunctionComponent<SourceCodeProps> = (props) => {
const editorRef = useRef<Parameters<EditorDidMount>[0] | null>(null);
const camelYamlDslSchema = useSchemasStore((state) => state.schemas.camelYamlDsl);
//const schemas = useSchemasStore((state) => state.schemas);
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
//const schemas = useSchemasStore((state) => state.schemas);

Comment on lines 9 to 10
import { isKamelet } from '../camel-utils/is-kamelet';
import { isIntegration } from '../camel-utils/is-integration';
Copy link
Member

Choose a reason for hiding this comment

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

[minor] could we add these tokens to the camel-utils/index.ts barrel file so we could have a single line bringing all the isXXX functions?

import { BaseCamelEntity } from '../models/camel-entities';
import { BaseVisualCamelEntity } from '../models/visualization/base-visual-entity';
import { CamelRoute, KameletBinding, Pipe } from '../models/visualization/flows';
import { Beans } from '../models/visualization/metadata';
import { EventNotifier, isDefined } from '../utils';
import { isKamelet } from '../camel-utils/is-kamelet';
import { isIntegration } from '../camel-utils/is-integration';
import { SourceSchemaType } from '../models/camel-entities/SourceSchemaType';
Copy link
Member

Choose a reason for hiding this comment

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

Probably this could come from camel-entities directly as well

Comment on lines 29 to 27
const [currentSchemaType, setCurrentSchemaType] = useState<SourceSchemaType>(SourceSchemaType.Route);
/** Set the Source Code and updates the Entities */

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
const [currentSchemaType, setCurrentSchemaType] = useState<SourceSchemaType>(SourceSchemaType.Route);
/** Set the Source Code and updates the Entities */
const [currentSchemaType, setCurrentSchemaType] = useState<SourceSchemaType>(SourceSchemaType.Route);
/** Set the Source Code and updates the Entities */

const entitiesHolder = rawEntities.reduce(
(acc, rawEntity) => {
const entity = getEntity(rawEntity);
setCurrentSchemaType(getEntityType(rawEntity));
Copy link
Member

Choose a reason for hiding this comment

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

[important] I think we don't need to set the schema inside of this iteration, as it leads to setting many times the same value, potentially firing subsequent updates.

I guess that it will be enough to inspect only the first element of the rawEntities array to determine which schema to apply just once.

Comment on lines 41 to 50
setSchema(name: string, schema: Schema) {
const type: SourceSchemaType = SourceSchemaType[name];
if (name === 'camelYamlDsl') {
this.config[SourceSchemaType.Route].schema = schema;
}
if (type) {
this.config[type].schema = schema;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I saw the TS issue that you mentioned and I made a few changes to the method:

  setSchema(name: string | SourceSchemaType, schema: Schema) {
    if (name === 'camelYamlDsl') {
      this.config[SourceSchemaType.Route].schema = schema;
      return;
    }

    if (isEnumType(name, SourceSchemaType)) {
      const type: SourceSchemaType = SourceSchemaType[name];
      this.config[type].schema = schema;
    }
  }

The isEnumType function is a special type of function, a Typeguard function, now called Type predicate

It's defined like the following:

function isEnumType<T extends object>(type: unknown, enumObject: T): type is keyof T {
  return typeof type === 'string' && (type as keyof typeof enumObject) in enumObject;
}

You could use that function and place it in a utils folder, maybe alongside of packages/ui/src/utils/is-enum-type.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Many thanks!

introduce the SourceSchemaType enum
@mmelko mmelko changed the title WIP: feat(schemas): Introduce EntitySchemaConfig and currentEntity feat(schemas): Introduce EntitySchemaConfig and currentEntity Oct 4, 2023
@mmelko mmelko changed the title feat(schemas): Introduce EntitySchemaConfig and currentEntity feat(CodeEditor): Changing schemas based on the content Oct 4, 2023
Copy link
Member

@lordrip lordrip left a comment

Choose a reason for hiding this comment

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

It looks great @mmelko , thanks for putting this together, I really like updating the schema upon writing, very cool.

@lordrip lordrip merged commit 9de45fa into KaotoIO:main Oct 4, 2023
2 checks passed
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