Skip to content

fix: prevent duplicate ID generation in fast multi-block renders #6621

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.

Sign up for GitHub

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

Open
binary8421 wants to merge 3 commits into
base: develop
Choose a base branch
from
+54 −6
Open

fix: prevent duplicate ID generation in fast multi-block renders #6621

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10347f0
fix: prevent duplicate ID generation in fast multi-block renders
binary8421 May 28, 2025
dbca4d1
[autofix.ci] apply automated fixes
autofix-ci[bot] May 28, 2025
131622e
Update packages/mermaid/src/utils.ts
binary8421 May 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions docs/config/setup/mermaid/interfaces/DetailedError.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,36 @@

# Interface: DetailedError

Defined in: [packages/mermaid/src/utils.ts:783](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L783)
Defined in: [packages/mermaid/src/utils.ts:829](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L829)

## Properties

### error?

> `optional` **error**: `any`

Defined in: [packages/mermaid/src/utils.ts:788](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L788)
Defined in: [packages/mermaid/src/utils.ts:834](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L834)

---

### hash

> **hash**: `any`

Defined in: [packages/mermaid/src/utils.ts:786](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L786)
Defined in: [packages/mermaid/src/utils.ts:832](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L832)

---

### message?

> `optional` **message**: `string`

Defined in: [packages/mermaid/src/utils.ts:789](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L789)
Defined in: [packages/mermaid/src/utils.ts:835](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L835)

---

### str

> **str**: `string`

Defined in: [packages/mermaid/src/utils.ts:784](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L784)
Defined in: [packages/mermaid/src/utils.ts:830](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/utils.ts#L830)
48 changes: 47 additions & 1 deletion packages/mermaid/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,52 @@ const d3CurveTypes = {

const directiveWithoutOpen =
/\s*(?:(\w+)(?=:):|(\w+))\s*(?:(\w+)|((?:(?!}%{2}).|\r?\n)*))?\s*(?:}%{2})?/gi;
const TimeProvider = {
_canUsePerformanceNow: false,
_lastTimestamp: 0,
_initialized: false,
_incrementStep: 1,

_initialize: function () {
if (this._initialized) {
return;
}

if (
window.performance &&
typeof window.performance.now === 'function' &&
window.performance.timing &&
typeof window.performance.timing.navigationStart === 'number'
) {
this._canUsePerformanceNow = true;
this._lastTimestamp = window.performance.timeOrigin * 1e3;
} else {
this._canUsePerformanceNow = false;
this._lastTimestamp = Date.now() * 1e3;
}
this._initialized = true;
},

_getRawTimestamp: function () {
if (this._canUsePerformanceNow) {
return Math.floor((window.performance.timeOrigin + window.performance.now()) * 1e3);
} else {
return Date.now() * 1e3;
}
},

getTimestamp: function () {
this._initialize();
const rawTimestamp = this._getRawTimestamp();

if (rawTimestamp <= this._lastTimestamp) {
this._lastTimestamp += this._incrementStep;
} else {
this._lastTimestamp = rawTimestamp;
}
return this._lastTimestamp;
},
};
/**
* Detects the init config object from the text
*
Expand Down Expand Up @@ -759,7 +805,7 @@ export class InitIDGenerator {
// TODO: Seed is only used for length?
// v11: Use the actual value of seed string to generate an initial value for count.
this.count = seed ? seed.length : 0;
this.next = deterministic ? () => this.count++ : () => Date.now();
this.next = deterministic ? () => this.count++ : () => TimeProvider.getTimestamp();
Copy link
Copy Markdown
Member

@sidharthv96 sidharthv96 May 28, 2025

Choose a reason for hiding this comment

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

Doing this should solve the problem of duplicate ID, and we won't have to add the complexity of TimeProvider.
We don't have a requirement that the ID should be the timestamp.

Suggested change
this.next = deterministic ? () => this.count++ : () => TimeProvider.getTimestamp();
this.next = deterministic
? () => this.count++
: () => Number.parseInt(`${Date.now()}${this.count++}`);

Or even simpler, we could concatenate it and return the string, we'd have to fix the tests to match the functionality though.

Copy link
Copy Markdown
Author

@binary8421 binary8421 May 30, 2025

Choose a reason for hiding this comment

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

Thank you for your input. From the perspective of long-term project maintenance, I’d like to explain why I believe TimeProvider is a suitable choice:

Timestamp Precision and Uniqueness:
By leveraging performance.now() for higher precision and ensuring monotonic increments, TimeProvider guarantees the generation of unique IDs.
Cross-Platform Compatibility:
TimeProvider is designed to work reliably across diverse environments, such as Node.js or older browsers, by handling differences in timing APIs.
Maintainability and Extensibility:
The modular design of TimeProvider encapsulates timestamp generation logic, making it easier to maintain and extend. If future requirements demand adjustments to timestamp precision or uniqueness strategies, changes can be made within TimeProvider without impacting the calling code.
Test Friendliness:
By building on the existing project structure, TimeProvider minimizes conceptual changes, allowing the current test cases to remain intact without requiring modifications.
I hope this clarifies the rationale behind using TimeProvider. If there are specific concerns or alternative approaches you’d like to explore, I’m happy to discuss further to find the best solution for the project!

Copy link
Copy Markdown
Member

@sidharthv96 sidharthv96 Jun 20, 2025

Choose a reason for hiding this comment

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

Are you using AI for this? 🤔

}
}

Expand Down
Loading