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

Fix exports of ogv-support.js/ogv-version.js + some JS build clean-up #620

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Mesoptier
Copy link

The main reason for this PR is to fix the issue where the compiled ogv-support.js and ogv-version.js files don't actually export anything, where you'd expect the OGVCompat and OGVVersion exports. 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 the window object when loaded in a <script> tag. Therefore, I've also removed the explicit window-assignments from the main three entrypoints.

See also: https://webpack.js.org/configuration/output/#type-umd


Other small changes to the JS build:

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant