fix(cli): throw warning for duplicate systems #2325
Conversation
🦋 Changeset detectedLatest commit: 7ee014b The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
vmaark
changed the title
| if (systems.some((elem, idx) => elem.systemId === item.systemId && idx !== index)) { | ||
| throw new Error(`Duplicate system ID ${item.systemId} found in the config.`); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Since we are working with objects, we could probably simplify this a bit by just checking if each item and elem are not the same (rather than their indices).
I think we could improve this error message too, something like:
Found two systems with the same system ID: {name1}, {name2}
System IDs are generated from the first 14 bytes of the name, so you may need to rename one of these systems to avoid the overlap.
There was a problem hiding this comment.
improved the error message.
I think it's safer to use the idx in the context of objects, a change in the logic could easily introduce a bug where you'd be checking e.q. the equality of different objects but with the same id (and they would never equal on the object reference level)
There was a problem hiding this comment.
in this case, we're just trying to make sure we're comparing system IDs, but we don't want to compare to itself, so an object reference seems fine
There was a problem hiding this comment.
re: above code, could potentially clarify with
const overlappingSystem = systems.find(...);
if (overlappingSystem) {
throw new Error(`Found two systems with the same system ID: ${system.name}, ${overlappingSystem.name}.\n\nSystem IDs are generated from the first 14 bytes of the name, so you may need to rename one of these systems to avoid the overlap.`);
}
|
Awesome, thanks for the quick turnaround! Looks like there's a merge conflict, which is keeping CI from running. |
vmaark
force-pushed
the
fix/warning-for-duplicate-system
branch
from
4304adb to
203d504
Compare
Resolves #2263
Throw error when systems with the same ids occur.