Fix exports of ogv-support.js/ogv-version.js + some JS build clean-up
#620
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I _think_ this is the desired behavior, since in my opinion it's weird to build a bunch of C code when running `npm install` with the expectation of just installing the node_modules. The command will now only run before `npm pack` and `npm publish`. See https://docs.npmjs.com/cli/v9/using-npm/scripts#life-cycle-scripts
…a UMD Previously, the compiled ogv-support.js and ogv-version.js files would not actually export anything in AMD/CommonJS and only expose directly on the global object.
This is now automatically handled by UMD when the scripts are loaded outside an AMD/CommonJS module environment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main reason for this PR is to fix the issue where the compiled
ogv-support.jsandogv-version.jsfiles don't actually export anything, where you'd expect theOGVCompatandOGVVersionexports. In fact, these files would only assign those values to the global object.I've fixed this by specifying the
output.library.type: 'umd'option for these entries in the webpack config. This will make the exports work as expected in AMD/CommonJS module environments.Additionally, by omitting
output.library.name, the exports will be assigned directly to thewindowobject when loaded in a<script>tag. Therefore, I've also removed the explicitwindow-assignments from the main three entrypoints.See also: https://webpack.js.org/configuration/output/#type-umd
Other small changes to the JS build:
prepublishscript toprepack, somake -j4 distonly runs beforenpm pack/npm publishand NOT fornpm install. This might require a change in the README. If you disagree with this change, I suggest changing it toprepareinstead as per https://docs.npmjs.com/cli/v9/using-npm/scripts#prepare-and-prepublish.package-lock.jsonafternpm install.modeis now specified per-script inpackage.json, allowing for development mode builds while developing.