feat: light esm build #765
Conversation
✅ Deploy Preview for cld-vp-esm-pages ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cld-video-player ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>Cloudinary Video Player</title> |
There was a problem hiding this comment.
Not critical but the title should match the example name ....same comment for the second example as well.
|
|
||
| <script type="module"> | ||
| import { videoPlayer } from 'cloudinary-video-player/light'; | ||
| import 'cloudinary-video-player/light/chapters'; |
There was a problem hiding this comment.
would this work import 'cloudinary-video-player/chapters'? if so i think its preferable since the current syntax implies that this is a light plugin which is not true in my eyes (The plugin is the same)
Also this way the imports are not consistent with the css import
There was a problem hiding this comment.
I agree, unfortunately it won't work since both /chapters and /light/chapters are importing videojs, but they were compiled using a different videojs core.
I tried multiple things to work around it but unfortunately nothing worked 100% in all of the full/light/esm/cdn variants :(
| const tracksArr = [textTracks.captions, ...(textTracks.subtitles || [])]; | ||
| return { | ||
| textTracks: hasConfig(textTracks), | ||
| textTracksLength: tracksArr.length, | ||
| textTracksOptions: hasConfig(textTracks.options) || Object.keys(textTracks.options).join(','), | ||
| textTracksOptions: hasConfig(textTracks.options) && Object.keys(textTracks.options).join(','), |
There was a problem hiding this comment.
how these two changes are related to exporting light version?
There was a problem hiding this comment.
They don't, just fixed a warning I got while working on this
| const webpackConfig = require('./es6.config'); | ||
| const path = require('path'); | ||
|
|
||
| delete webpackConfig.entry; // overwrite |
There was a problem hiding this comment.
could be that there is something in webpack-merge api that allows you to overwrite when merging which maybe can be easier to read as it is more explict ? (Just a thought, i did not really go and checked their API)
There was a problem hiding this comment.
I thought delete webpackConfig.entry would be as explicit as it gets, what could be more explicit than that? :)
No description provided.