Use const enum to have readable glyph names that are inlined as unicode code points and unicode strings during compilation. #30

ronyeh · 2023-07-24T10:03:38Z

This is a work in progress as I have discovered some issues with our use of const enums, and how it may conflict with the way we compile our library.

Anyways.... at least in the ES modules output, this works perfectly fine.

This PR adds two const enums, SMuFLGlyph and SMuFLCodePoint, which contain all the smufl glyphs and hex code points.

The big idea is that in our code, we can use readable names like: SMuFLGlyph.csymMinor

The compiled output should ideally make no reference to these enums. All uses of the enums should be erased and replaced by their values (similar in spirit to C preprocessor #define constants).

So SMuFLGlyph.csymMinor => "\uE874".

I have verified that this works great in the ESM build of our library. This means we do not need to use String.fromCharCode(), which allows us to avoid the issues noted in #29 .

By using these const enums, we will be using the unicode characters directly. No extra step in translating the code points to characters.

If for some reason we really do need the code point, this PR includes a const enum of all the code points (SMuFLCodePoint).

I'd like to hear any and all comments... both for an against this approach. :-)

unicode code points and unicode strings during compilation.

ronyeh · 2023-07-24T10:07:58Z

The biggest drawback right now is that our webpack exports do not erase the const enums... and the enums are treated as regular enums.

We have used const enums in the past.... so it's possible that we haven't been getting the "full performance benefit" of those const enums.

It is possible that our efforts to speed up our build (by setting transpileOnly: true) is breaking the const enums. See: microsoft/TypeScript#16671 (comment)

Regardless, it would be super cool if we could figure this out.

Maybe the right answer is to emit ES modules first, and then run the webpack build process on the ESM files? That way, we are "compiling" the ESM files, which already have the const enums erased.

ronyeh · 2023-07-24T10:09:32Z

Even if we don't go with the const enum approach in the end, I believe we can avoid converting code points to strings in most cases.

We should just use unicode string literals like: '\uE874'.

ronyeh · 2023-07-24T10:11:13Z

Remember that the '\uXXXX' escape code only works for code points less than or equal to 0xFFFF. For bigger code points, we need to use this form:

'\u{1d12b}'
// accidentalDoubleFlat alternate code point is > 0xFFFF

ronyeh · 2023-07-24T10:13:06Z

src/typeguard.ts

@@ -8,7 +8,7 @@ import { Dot } from './dot';
 import { GraceNote } from './gracenote';
 import { GraceNoteGroup } from './gracenotegroup';
 import { Note } from './note';
-import { RenderContext } from './rendercontext.js';
+import { RenderContext } from './rendercontext';


This fixes an error in the import. It should not include the .js extension.

ronyeh · 2023-07-24T10:13:50Z

src/chordsymbol.ts

@@ -360,7 +361,7 @@ export class ChordSymbol extends Modifier {
      symbolModifier: SymbolModifiers;
    }> = {}
  ): this {
-    return this.addText(String.fromCharCode(0xe874 /*csymMinor*/, 0xe874 /*csymMinor*/), params);
+    return this.addText(SMuFLGlyph.csymMinor + SMuFLGlyph.csymMinor, params);


In the ESM build, this line is successfully changed to:

return this.addText("\uE874" + "\uE874", params);

ronyeh · 2023-07-24T10:14:30Z

src/vibrato.ts

@@ -4,7 +4,6 @@
 import { Bend } from './bend';
 import { Modifier } from './modifier';
 import { ModifierContext, ModifierContextState } from './modifiercontext';
-import { RenderContext } from './rendercontext';


Unused import.

ronyeh · 2023-07-24T10:15:42Z

tests/vibrato_tests.ts

@@ -67,11 +69,11 @@ function harsh(options: TestOptions, contextBuilder: ContextBuilder): void {
        { str: 4, fret: 9 },
      ],
      duration: 'h',
-    }).addModifier(new Vibrato().setVibratoCode(0xeae2), 0),
+    }).addModifier(new Vibrato().setVibratoCode(SMuFLCodePoint.wiggleVibratoLargeFastest /* 0xeae2 */), 0),


@rvilarl Should we be relying on code points? Why not just accept the unicode character directly?

In this case, we would use: '\ueae2'

rvilarl · 2023-07-24T16:49:35Z

Hi Ron,
I agree that we should move to the scape codes. The enums are no doubt easier to understand but we should make sure that the size of the library does not grow too much. The new file is very big.

ronyeh · 2023-07-24T18:47:51Z

I will make sure this approach does not add any size to the library. The idea behind const enum is that it is supposed to be erased by TypeScript during compilation, and replaced by the constant escape code. Anything that is unused should be completely erased. However, I inspected the webpack output and there is some evidence of some parts of the enum remaining. The ESM output is perfect, as I expected. Needs more investigation. In the meantime, I will suggest using the \u and \u{} escape codes directly, and avoiding String.fromCharCode if possible. We can just put the comment with the smufl glyph name next to each escape code, as you have been doing. That's probably good enough for now.

…

On Mon, Jul 24, 2023, 9:49 AM Rodrigo Vilar ***@***.***> wrote: Hi Ron, I agree that we should move to the scape codes. The enums are no doubt easier to understand but we should make sure that the size of the library does not grow too much. The new file is very big. — Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2MCMO6FOBPTTM5W4KKFTXR2RSTANCNFSM6AAAAAA2VK2P4U> . You are receiving this because you authored the thread.Message ID: ***@***.***>

ronyeh · 2023-07-24T20:10:45Z

https://github.com/evanw/esbuild/blob/main/CHANGELOG-2021.md#0149

Maybe we will have to switch from webpack to esbuild to make it work.

There might be a webpack plugin, but I am also a fan of esbuild for other reasons so maybe it is worth the trouble.

ronyeh · 2023-08-01T06:59:35Z

Closing this one for now, but I have been investigating esbuild in another branch and it is very promising.

It's fast.
It seems to handle const enums in the way we expect.

Use const enum to have readable glyph names that are inlined as

d718822

unicode code points and unicode strings during compilation.

ronyeh commented Jul 24, 2023

View reviewed changes

ronyeh closed this Aug 1, 2023

ronyeh mentioned this pull request Sep 3, 2023

Use friendly names for SMUFL glyphs (e.g., use a const enum for codepoints) #23

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use const enum to have readable glyph names that are inlined as unicode code points and unicode strings during compilation. #30

Use const enum to have readable glyph names that are inlined as unicode code points and unicode strings during compilation. #30

ronyeh commented Jul 24, 2023

ronyeh commented Jul 24, 2023

ronyeh commented Jul 24, 2023

ronyeh commented Jul 24, 2023 •

edited

ronyeh Jul 24, 2023

ronyeh Jul 24, 2023

ronyeh Jul 24, 2023

ronyeh Jul 24, 2023

rvilarl commented Jul 24, 2023

ronyeh commented Jul 24, 2023 via email

ronyeh commented Jul 24, 2023 •

edited

ronyeh commented Aug 1, 2023

Use const enum to have readable glyph names that are inlined as unicode code points and unicode strings during compilation. #30

Use const enum to have readable glyph names that are inlined as unicode code points and unicode strings during compilation. #30

Conversation

ronyeh commented Jul 24, 2023

ronyeh commented Jul 24, 2023

ronyeh commented Jul 24, 2023

ronyeh commented Jul 24, 2023 • edited

ronyeh Jul 24, 2023

Choose a reason for hiding this comment

ronyeh Jul 24, 2023

Choose a reason for hiding this comment

ronyeh Jul 24, 2023

Choose a reason for hiding this comment

ronyeh Jul 24, 2023

Choose a reason for hiding this comment

rvilarl commented Jul 24, 2023

ronyeh commented Jul 24, 2023 via email

ronyeh commented Jul 24, 2023 • edited

ronyeh commented Aug 1, 2023

ronyeh commented Jul 24, 2023 •

edited

ronyeh commented Jul 24, 2023 •

edited