(fix) O3-3875: JSON editor in the form builder does not show an error indicator#1085

Open

RajPrakash681 wants to merge 3 commits intoopenmrs:mainfrom

RajPrakash681:fix-schema-editor-error-indicators

Contributor

RajPrakash681 commented Feb 17, 2026 •

edited

Loading

Requirements

This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
My work includes tests or is validated by existing tests.

Summary

a fix to implement error indicators that are not being shown in dev3 or similar ones

Screenshots

Related Issue

https://issues.openmrs.org/browse/O3-3875

Other

NA

Contributor Author

RajPrakash681 commented Feb 17, 2026 •

edited

Loading

Hey @ibacher, thanks for the feedback on the CDN usage completely understood.

The root cause was that ace-builds/webpack-resolver (which dynamically resolves ace assets at runtime) fails in the micro-frontend deployment environment because it computes paths that don't exist in the deployed bundle. The CDN was a workaround for that, but obviously not acceptable.

The proper fix:

Replaced the CDN and webpack-resolver with explicit static imports ace-builds/src-noconflict/mode-json and theme-textmate are now bundled at build time, so no runtime network requests are needed
Set useWorker: false to prevent ace from trying to fetch the JSON worker file at runtime. Since the ace worker is what provided the gutter error icons automatically, we now pass those manually via the annotations prop, so the behaviour is identical
Everything ships with the app bundle and works fully offline

ibacher requested changes

View reviewed changes

src/components/form-editor/form-editor.component.tsx Outdated

Comment on lines +319 to +303

		{!schema ? (
		{schema ? null : (

Member

ibacher Feb 20, 2026

I know Sonar flags this, but Sonar is wrong and this is less readable.

src/components/form-editor/form-editor.component.tsx Outdated

                                   onClick={handleCopySchema}
                                 />
-                                <a download={`${form?.name}.json`} href={window.URL.createObjectURL(downloadableSchema)}>
+                                <a download={`${form?.name}.json`} href={globalThis.URL.createObjectURL(downloadableSchema)}>

Member

ibacher Feb 20, 2026

Ditto this. window.URL is fine.

src/components/form-editor/form-editor.component.tsx Outdated

                   <div className={styles.backButton}>
-                    <ConfigurableLink to={window.getOpenmrsSpaBase() + 'form-builder'}>
+                    <ConfigurableLink
+                      to={(globalThis as unknown as { getOpenmrsSpaBase: () => string }).getOpenmrsSpaBase() + 'form-builder'}

Member

ibacher Feb 20, 2026

And this is why Sonar is wrong. we explicitly use window here because it's already properly typed.

src/components/schema-editor/schema-editor.component.tsx Outdated

+                path: string,
+                suggestions: Array<{ name: string; type: string; path: string }>,
+              ) => {
+                if (schemaProps && typeof schemaProps === 'object') {

Member

ibacher Feb 20, 2026

Suggested change

      
              if (schemaProps && typeof schemaProps === 'object') {
          
              if (schemaProps && typeof schemaProps === 'object' && !Array.isArray(schemaProps)) {

Member

ibacher Feb 20, 2026

typeof blah === 'object' basically tells you that it isn't one of a handful of types. It doesn't tell you that's it's necessarily a "normal" JS object.

src/components/schema-editor/schema-editor.component.tsx Outdated

+                    const currentPath = path ? `${path}.${propertyName}` : propertyName;
+                    const typedProperty = property as SchemaProperty;
+                    if (typeof property === 'object') {

Member

ibacher Feb 20, 2026

Suggested change

      
                  if (typeof property === 'object') {
          
                  if (typeof property === 'object' && !Array.isArray(property)) {

src/components/schema-editor/schema-editor.component.tsx Outdated

                   addCompleter({
-                    getCompletions: function (editor, session, pos, prefix, callback) {
+                    getCompletions: function (
+                      _editor: unknown,

Member

ibacher Feb 20, 2026

Not sure I understand why you're adding the _ prefix? Also I always think these things read better as:

function getCompletions(editor: unknown...) {

};

This is, after all, just a standard JS function in a standard JS object.

src/components/schema-editor/schema-editor.component.tsx Outdated

                 useEffect(() => {
                   addCompleter({
-                    getCompletions: function (editor, session, pos, prefix, callback) {
+                    getCompletions: function (

Member

ibacher Feb 20, 2026

Incidentally, you should be able to avoid all these typings completely by just by using satisfies Completer (Completer is the Ace type this is implementing). That should give us something fully typed... I think.

src/components/schema-editor/schema-editor.component.tsx

    
                    const traverse = (schemaPath) => {

                      const pathSegments = schemaPath.split('/').filter((segment) => segment !== '' || segment !== 'type');

                    const traverse = (schemaPath: string) => {

                      const pathSegments = schemaPath.split('/').filter((segment) => segment !== '' && segment !== 'type');

Member

ibacher Feb 20, 2026

Why this change?

Contributor Author

RajPrakash681 Feb 20, 2026 •

edited

Loading

Why this change?

The original condition with || is always true, so it never actually filters anything out:

If segment is '': segment !== 'type' is true → condition passes
If segment is 'type': segment !== '' is true → condition passes
Any other value: both are true → condition passes

Using && correctly filters out segments that are either empty strings or equal to 'type'.

src/components/schema-editor/schema-editor.component.tsx

                       let lineNumber = -1;
                       for (const segment of pathSegments) {
-                        if (segment === 'properties' || segment === 'items') continue; // Skip 'properties' and 'items'

Member

ibacher Feb 20, 2026

Why are we removing comments?

src/components/schema-editor/schema-editor.component.tsx

    
                      for (const segment of pathSegments) {

                        if (segment === 'properties' || segment === 'items') continue; // Skip 'properties' and 'items'

                        const match = segment.match(/^([^[\]]+)/); // Extract property key

Member

ibacher Feb 20, 2026

???


          (fix) O3-3875: remove CDN dependency from schema editor ace configura…

8b8ce15

…tion

Replace runtime CDN asset loading with bundled imports for ace-builds
mode, theme, and language tools. Set useWorker: false to prevent ace
from fetching the JSON worker file at runtime. Add gutter annotations
alongside existing markers so error icons render correctly in production.

Also remove unused isLoading prop, fix pre-existing logic bug in traverse
filter (|| -> &&), extract sub-components outside SchemaEditor, and
resolve SonarQube warnings in both schema-editor and form-editor.

RajPrakash681 force-pushed the fix-schema-editor-error-indicators branch from dd3ffc5 to 1cf6e45 Compare

February 20, 2026 22:14


           fix readability and type safety issues

dc94a88

RajPrakash681 force-pushed the fix-schema-editor-error-indicators branch from 1cf6e45 to dc94a88 Compare

February 20, 2026 22:22

RajPrakash681 requested a review from ibacher

February 20, 2026 22:25


          Merge branch 'main' into fix-schema-editor-error-indicators

7ecfe42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet