Update classId generation to use generic type

[jgreywolf]

📑 Summary

In order to allow class diagrams where there are multiple generic classes with the same name, only different type parameter, updated the classId generation logic to append -TYPE to the end of class name to get the id.

However, this means that any later referrals to that class needs to either be in the format class~T~ or class-T to ensure that the correct class is utilized.

• Updated documentation
• added tests

Resolves #4762

📏 Design Decisions

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added necessary unit/e2e tests.
• 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
• 🔖 targeted develop branch

[netlify]

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	234a2cc
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/65ffcfa1867cc50008bc85c0
😎 Deploy Preview	https://deploy-preview-5223--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[sidharthv96]

Nice! This should help out a lot of people.

Is this a breaking change though? (Or did the previous implementation not support this at all?)
Should we fall back to previous behavior, so existing diagrams don't break?

If the diagram refers class without type, we could pick whichever class was defined first (or whatever the current behaviour is)?

We need rendering tests as well.

cc @knsv

[jgreywolf]

Nice! This should help out a lot of people.

Is this a breaking change though? (Or did the previous implementation not support this at all?) Should we fall back to previous behavior, so existing diagrams don't break?

If the diagram refers class without type, we could pick whichever class was defined first (or whatever the current behaviour is)?

We need rendering tests as well.

cc @knsv

The previous implemetation did not support this at all. This should not be a breaking change, any existing class diagrams will still parse the same as they did before

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 48 lines in your changes missing coverage. Please review.

Project coverage is 5.72%. Comparing base (c00bf26) to head (234a2cc).
Report is 1720 commits behind head on develop.

Files with missing lines	Patch %	Lines
packages/mermaid/src/diagrams/class/classDb.ts	0.00%	47 Missing ⚠️
packages/mermaid/src/diagrams/class/classTypes.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #5223      +/-   ##
==========================================
- Coverage     5.72%   5.72%   -0.01%     
==========================================
  Files          277     277              
  Lines        41896   41902       +6     
  Branches        27      27              
==========================================
  Hits          2400    2400              
- Misses       39495   39501       +6     
  Partials         1       1              

Flag	Coverage Δ
unit	5.72% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/mermaid/src/diagrams/class/classTypes.ts	0.00% <0.00%> (ø)
packages/mermaid/src/diagrams/class/classDb.ts	0.00% <0.00%> (ø)

[jgreywolf]

We need rendering tests as well.

There are already rendering tests that cover use of generic in class name

[jgreywolf]

@sidharthv96
FYI - in reviewing the failing rendering tests, these are actually expected, since this is fixing a bug and the previous renders were actually not showing the correct data

This is assuming that the test result images are showing the old (expected) on the left, and the new (received) is on the left

[sidharthv96]

That's fine. Yes it's like you mentioned.

[sidharthv96]

The previous implemetation did not support this at all. This should not be a breaking change, any existing class diagrams will still parse the same as they did before

Consider this diagram

classDiagram
    class Class01~T~
      Class01 : size()
      Class01 : int chimp
      Class01 : int gorilla
      Class08 <--> C2: Cool label
      class Class10~T~ {
        &lt;&lt;service&gt;&gt;
        int id
        test()
      }

In mermaid.live

In branch preview

So this infact breaks existing diagrams. So we should add a fallback so the diagram will render the same.

[jgreywolf]

FYI - I am redoing this slightly as part of a separate PR (too many conflicts). Im not sure this is the best solution anymore either. I mentioned to someone else recently, that I totally regret using ~~ to denote types.