Premium support for our pure JavaScript UI components


Post by gbrdhvndi »

Hello there,

For a number of reasons we are building our own component bundle from source so that we could use Bryntum components in Salesforce Lightning Experience.

We use Rollup.js to bundle together SchedulerPro (which we recently replaced with Gantt) and Calendar products.

It has been a straightforward experience until we added TaskBoard into the mix.

Now our build is complaining about class name conflicts which we can't resolve with simple product namespace prefixing as we did before, since TaskBoard is not an extension of Gantt but rather an entirely different code branch with Scheduler being the common ancestor for all three included products (Gantt, Calendar, and TaskBoard).

The following screenshot shows the Rollup build log.

Screenshot 2021-10-12 at 00.48.14.png
Screenshot 2021-10-12 at 00.48.14.png (107.92 KiB) Viewed 1918 times

As you can see, a number of classes was flagged as they were exported from both Gantt and TaskBoard namespaces: ProjectModel, TaskDrag, TaskEdit, TaskEditor, TaskMenu, TaskModel, TaskNavigation, TaskStore, and TaskTooltip.

Please advise if there are any workarounds.

Many thanks!

Aleksei


Post by Maxim Gorkovsky »

Hello.
As far as I understand you can import them with prefixed name and export prefixed too. As long as you remember the renamed classes for later use - should work like charm. In our modules bundles we have similar name collision for many classes, like depenendency feature. So we import it with a prefix:

import SchedulerDependencies from 'lib/Scheduler/features/Dependencies.js';
import Dependencies from 'lib/Gantt/features/Dependencies.js';

So I suggest you to try smth similar:

import TBTaskDrag from 'lib/TaskBoard/features/TaskDrag.js';
import TaskDrag from 'lib/Gantt/features/TaskDrag.js';

and if you find any problems, please let us know and we will see how to work around them.


Post by gbrdhvndi »

Thank you, Maxim. I wish there was another way.

We already prefix classes with their respective product names if they are being superseded up the dependency tree.

For example:

// Grid imports
import GridCellEdit from './Grid/feature/CellEdit';
import GridGroupSummary from './Grid/feature/GroupSummary';

// Scheduler imports
import SchedulerAssignmentModel from './Scheduler/model/AssignmentModel';
import GroupSummary from './Scheduler/feature/GroupSummary';

// SchedulerPro imports
import SchedulerProAssignmentModel from './SchedulerPro/model/AssignmentModel';
import SchedulerProTaskEdit from './SchedulerPro/feature/TaskEdit';

// Gantt imports
import AssignmentModel from './Gantt/model/AssignmentModel';
import CellEdit from './Gantt/feature/CellEdit';
import TaskEdit from './Gantt/feature/TaskEdit';

// Calendar imports
// ...

export {
	GridCellEdit,
	GridGroupSummary,
	SchedulerAssignmentModel,
	GroupSummary,
	SchedulerProAssignmentModel,
	SchedulerProTaskEdit,
	AssignmentModel,
	CellEdit,
	TaskEdit,
	// etc.
};

Aliasing feels right and looks well when classes accessed by their documented (original) name are still working as expected. For instance, the AssignmentModel class instance would probably work just fine with SchedulerPro even though it belongs in Gantt. In exceptional circumstances developers can use the prefixed version, of course.

However, prefixing top level classes does not feel right as it has a potential for a breaking change.

For example, the TaskModel class is exported by both Gantt and TaskBoard components. If we prefix the Gantt one and export it as GanttTaskModel then it's a breaking change as the new TaskModel class is not backwards compatible with Gantt.

Same logic could be applied the other way around. If we start prefixing classes in the TaskBoard namespace and one day a new conflict is introduced, then it is another breaking change.

I wonder if we should rearrange our exports into nested namespaces like so:

window.bryntum = {
	Core: {},
	Grid: {},
	Scheduler: {},
	SchedulerPro: {},
	Gantt: {},
	Calendar: {},
	TaskBoard: {},
};

Then any potential name collisions would be eliminated automatically:

{
	Scheduler: {
		feature: { EventEdit },
		data: { AssignmentStore },
		model: { AssignmentModel },
	},
	SchedulerPro: {
		feature: { TaskEdit },
		data: { AssignmentStore },
		model: { AssignmentModel },
	},
	Gantt: {
		feature: { CellEdit, TaskEdit },
		data: { AssignmentStore },
		model: { AssignmentModel },
	},
	Calendar: {
		feature: { EventEdit },
	},
	TaskBoard: {
		feature: { TaskEdit },
	},
}

Thoughts?

Aleksei


Post by Maxim Gorkovsky »

For example, the TaskModel class is exported by both Gantt and TaskBoard components. If we prefix the Gantt one and export it as GanttTaskModel then it's a breaking change as the new TaskModel class is not backwards compatible with Gantt.

As far as I understand, rollup handles that pretty well. It should identify classes not by the name you import it to the entry, but by the path. I modified entry file:

// This class could now only be imported like `import { GanttTaskModel } from 'bundle.js'`
export { default as GanttTaskModel } from '../lib/Gantt/model/TaskModel.js';

and in the bundle I'm still getting original name:

class TaskModel extends GanttEvent.derive(TimeSpan).mixin(PartOfProject$1, PercentDoneMixin) {..}

// and later it is used like
taskModelClass : TaskModel

As for duplicated imports, they look like this:

// entry.js
export { default as SchedulerDependencies } from '../lib/Scheduler/feature/Dependencies.js';
export { default as Dependencies } from '../lib/Gantt/feature/Dependencies.js';

// bundle.js

// Scheduler deps feature
class Dependencies extends InstancePlugin.mixin(...) {..}

// Gantt deps feature
class Dependencies$1 extends Dependencies {...}

It means rollup can handles classes not by class name, but by module address which makes it impossible to use wrong class instance in the bundle.

However, prefixing top level classes does not feel right as it has a potential for a breaking change.

You can safely reexport documented classes. If we change or remove them, first we add a deprecation note and then remove in major releases only. We also add breaking changes section to changelog which specifies what was done and how to adjust your code. Between minor/patch releases you're safe.
We prefix top level classes too - models, stores, crudmanager, features, widgets. JS build tools make sure classes never have name collisions inside the bundle.

For example, the TaskModel class is exported by both Gantt and TaskBoard components. If we prefix the Gantt one and export it as GanttTaskModel then it's a breaking change as the new TaskModel class is not backwards compatible with Gantt.

I'm not sure I understand this one. You don't really break anything. Except for applications that were built for gantt bundle and were expecting TaskModel is a gantt task model. Which now use a new bundle where TaskModel is a taskboard task model. In this case I think it makes more sense to prefix whatever classes are used less extensively and easier to replace - likely TaskBoard's.

Does this answer your question?


Post by gbrdhvndi »

I'm not sure I understand this one. You don't really break anything. Except for applications that were built for gantt bundle and were expecting TaskModel is a gantt task model. Which now use a new bundle where TaskModel is a taskboard task model.

That's exactly what I meant in regards to name conflicts and potential breaking changes.

Yes, inside the bundle all class references are handled without issues. It's the outside world I'm worried about as our custom bundle is consumed by a number of product teams and refactoring code due to breaking changes in our library is very costly so we try to make as few of those as possible.

In this case I think it makes more sense to prefix whatever classes are used less extensively and easier to replace - likely TaskBoard's.

I have some ideas that might work. Thank you for your response.

Aleksei


Post by Maxim Gorkovsky »

It's the outside world I'm worried about as our custom bundle is consumed by a number of product teams and refactoring code due to breaking changes in our library is very costly so we try to make as few of those as possible.

That is understandable. So far there have been few major API updates which would require you to update entry file had you use it at the time:

  • in 3.0.1we renamed Common to Core which affected path in entry, but class names were untouched
  • in 4.0.0 we removed Core/adapter classes which were used to register widgets and moved that functionality to Widget class
  • in 4.0.0 TaskContextMenu was renamed to TaskMenu

But as far as I can tell, transition could go fairly smoothly withouth much things breaking. Some things renamed, smth changed, nothing went from public to not existent (except for the Core/adapter in 4.0). You can check log here: https://www.bryntum.com/products/gantt/changelog/?id=4.3.0

So, within major release you should be safe, and between major releases there are always API changes requiring attention.

I hope this information helped you to make decision.


Post by gbrdhvndi »

I've encountered another issue with our custom bundle.

If any components are imported after Gantt (for example, the Calendar or the TaskBoard) then the resulting UMD (non-LWC) module bundle throws an error on load:

Screenshot 2021-10-13 at 05.29.07.png
Screenshot 2021-10-13 at 05.29.07.png (29.63 KiB) Viewed 1891 times

This happens when the next component in line is trying to register its own locale by extending what's already there. And since Gantt excludes a number of locale properties, those are removed and any subsequent call to LocaleHelper.trimLocale() will fail.

I can manage around this by importing Gantt at the very end, after Calendar and TaskBoard. But I'd like to understand the reason for excluding those properties.

// Exclude not used properties
const exclude = {
    EventEdit : {
        Repeat : ''
    },
    RecurrenceCombo              : {},
    RecurrenceConfirmationPopup  : {},
    RecurrenceDaysCombo          : {},
    RecurrenceEditor             : {},
    RecurrenceFrequencyCombo     : {},
    RecurrenceLegend             : {},
    RecurrencePositionsCombo     : {},
    RecurrenceStopConditionCombo : {}
};

Are they no longer in use by Scheduler and SchedulerPro? How does the Scheduler(Pro) component continue to operate without these labels? Should we stop shipping our own translations for them?

Many thanks!

Aleksei


Post by alex.l »

I do see those properties in Scheduler(Pro) locales, as example "Repeat" string used for recurrenceCombo when you edit recurrent event. Same for others - used only if recurrent event feature in use. But EventEdit is empty in Gantt since Gantt uses TaskEvent feature instead.
I would suggest you to try to merge locales instead. https://bryntum.com/docs/gantt/api/Core/localization/LocaleHelper#function-mergeLocales-static

All the best,
Alex


Post by Maxim Gorkovsky »

If any components are imported after Gantt (for example, the Calendar or the TaskBoard) then the resulting UMD (non-LWC) module bundle throws an error on load

Do I understand correctly that you're talking about order of imports in an entry file that you use to create a UMD bundle? We never seen this kind of error before, because in our bundles we use a specific order of imports. For gantt it is: core, grid, scheduler, schedulerpro, gantt. For task board it is: core, grid, scheduler, taskboard. And I don't think we even have bundle that incorporates gantt + calendar + taskboard.
This looks like smth for us to investigate. I will add a note to this ticket about multi-product bundle: https://github.com/bryntum/support/issues/2076


Post by gbrdhvndi »

That is correct, it's the order of imports that matters. And I guess you've never seen this before because you never bundle all your products together.

As far as I can tell, Gantt is the only one with excluded locale properties. If you do something like that in Grid or Scheduler, the issue will be more apparent.

Aleksei


Post Reply