Skip to content
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.

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

Improve build process #1205

Merged
merged 2 commits into from
Nov 1, 2021
Merged

Improve build process #1205

merged 2 commits into from
Nov 1, 2021

Conversation

ronyeh
Copy link
Collaborator

@ronyeh ronyeh commented Oct 30, 2021

Generate .d.ts files (tsconfig.json setting).

Generate a vexflow.module.js file (that ends in export default Vex;).

Add flow-headless-browser.html to allow the qunit grunt task to run a streamlined vexflow-tests.js. The full flow.html will include things like async loading of web fonts, etc. The command line tests will not preload web fonts.

Gruntfile improvements to allow for dynamic font loading.

Delete jQuery file. Update glyphs.html to not require jQuery.

The output JS files are now as follows:

vexflow.js                <-- the production library with all fonts
vexflow.module.js         <-- same as above, but with export default Vex; at the bottom.
vexflow-debug.js          <-- non-minimized version of vexflow.js
vexflow-tests.js          <-- includes vexflow-debug.js, and all the test cases.

vexflow-core.js           <-- slim version of vexflow without any music fonts. It will dynamically load the font libraries below...
vexflow-font-bravura.js
vexflow-font-custom.js
vexflow-font-gonville.js
vexflow-font-petaluma.js

This was extracted out of the mega Font PR #1163

@ronyeh ronyeh mentioned this pull request Oct 30, 2021
@ronyeh
Copy link
Collaborator Author

ronyeh commented Oct 30, 2021

There's one thing I'd like your feedback on. I shortened the names of the output JS files in this PR. Is everyone okay with removing the "-min" from the minified files? Our files are all minified, except for *-debug.js and *-tests.js.

vexflow-full-min.js               => vexflow.js

vexflow-core-min.js               => vexflow-core.js
vexflow-font-bravura-min.js       => vexflow-font-bravura.js
vexflow-font-custom-min.js        => vexflow-font-custom.js
vexflow-font-gonville-min.js      => vexflow-font-gonville.js
vexflow-font-petaluma-min.js      => vexflow-font-petaluma.js
                                  => vexflow.module.js // a new file
vexflow-debug.js // unchanged
vexflow-tests.js // unchanged

Tone.js also does this. For example, this is minified: https://unpkg.com/tone/build/Tone.js

@0xfe
Copy link
Owner

0xfe commented Oct 30, 2021

The filename renaming is fine with me -- it does make more sense since min is the default.

Also, glad to be rid of jQuery.

@ronyeh ronyeh mentioned this pull request Oct 31, 2021
@0xfe
Copy link
Owner

0xfe commented Oct 31, 2021

Change looks good. One conflict after all those merges to fix :-) LMK when this is good to go.

    to run a streamlined vexflow-tests.js. The full flow.html will
    include things like async loading of web fonts, etc.
Generate .d.ts files.
Gruntfile improvements to allow for dynamic font loading.
Delete jQuery file. Update glyphs.html to not require jQuery.
@ronyeh ronyeh force-pushed the build branch 2 times, most recently from 0f5707a to 7427ff0 Compare November 1, 2021 08:47
Remove glyphs.html because the updated one was introduced in a
different PR.
@ronyeh
Copy link
Collaborator Author

ronyeh commented Nov 1, 2021

Fixed conflicts. Good to go.

Once this is merged, I'll rebase my Font PR to see where we're at. :-) The remaining Font related changes cut across much of the VexFlow code base, so I'll try to figure out the best way for you to review (perhaps by writing a "how to review" document, haha). Or maybe I can extract one more PR.

I've been working with Rodrigo to iron out any last migration issues with my Fonts branch. We're down to a single (albeit tricky) issue.

@0xfe
Copy link
Owner

0xfe commented Nov 1, 2021

Thanks Ron. Merging.

@0xfe 0xfe merged commit fe20e5b into 0xfe:master Nov 1, 2021
@ronyeh ronyeh deleted the build branch March 2, 2022 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants