search

muxinc / videojs-mux-kit

MIT License
33 stars 11 forks source link

SSR and Build fixes #14

Closed erikpena closed 3 years ago

erikpena commented 3 years ago

Pin unreleased videojs-vtt.js

dylanjha commented 3 years ago

Exchanged DM's with @erikpena 7.12.1 is the version we need, and that is a pre-release version. The latest release that NPM knows about is 7.11.8. Since we need the pre-release version we should pin it to that and then whenever a non pre-release version comes out (7.12.2 ? 7.12.3 ? ) then we can update the dep to ^7.12.

mmcc commented 3 years ago

Yep, sounds good to me.

On Wed, Apr 14, 2021, at 10:36 AM, Erik Peña wrote:

@.**** commented on this pull request.

In webpack.common.js https://github.com/muxinc/videojs-mux-kit/pull/14#discussion_r613445578:

  • alias: {
  • 'video.js': 'video.js/core',
  • },
  • },
  • module: {
  • rules: [
  • {
  • test: /.js$/,
  • loader: 'esbuild-loader',
  • }
  • ]
  • },
  • optimization: {
  • minimize: true,
  • minimizer: [new ESBuildMinifyPlugin({
  • target: 'es2015'

Yea, looks like it's still required. Here's what I'm presented with for Next.js when I remove the target—

Screen Shot 2021-04-14 at 10 29 15 AM https://user-images.githubusercontent.com/2187347/114753681-9a5ebb00-9d0c-11eb-9942-601e4320eb34.png

To be honest, I think we want to define the target because ESBuild defaults to esnext (https://esbuild.github.io/api/#target).

Looking at browser support for esnext, the result looks spotty— https://kangax.github.io/compat-table/esnext/

For ultra compatibility, I think it would behoove use target es2015.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/muxinc/videojs-mux-kit/pull/14#discussion_r613445578, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGCC3H6UVRXW3THIYZNVKTTIXHA7ANCNFSM424UAWQQ.

gkatsev commented 3 years ago

npm knows about 7.12.1 just will give you 7.11.8 by default. Setting the version to ^7.12.1 should work, you can also do npm i video.js@7.12.

erikpena commented 3 years ago

Hey @gkatsev, Actually I like that better because it is more succinct and gets me exactly what I'm after (7.12.1 <-> 8.0.0).