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

r.assert is not a function error on npm package #5218

Closed
alexlives opened this issue Feb 9, 2023 · 5 comments · Fixed by #5241
Closed

r.assert is not a function error on npm package #5218

alexlives opened this issue Feb 9, 2023 · 5 comments · Fixed by #5241
Milestone

Comments

@alexlives
Copy link

What do you want to do with Hls.js?

We're using the latest npm build for our React project. We're seeing the library throw an error when in production mode. It appears that references to assert are not being stripped out. We are now using the babel-plugin-transform-console plugin in our production build to remove assert. However, this feels like a workaround based on debugging the hls.js library.

Is it possible for the npm package installed to have console.assert removed so that the consuming client does not require any special plugin?

What have you tried so far?

hls.js 1.3.1
webpack 5

const optimization = {
  minimize: true,
  minimizer: [
    new TerserPlugin({
      terserOptions: {
        format: {
          comments: false,
        },
        extractComments: "all",
        compress: {
          drop_console: true,
        },
      },
    }),
  ],
};

drop_console: true seems to have no effect

@alexlives alexlives added Needs Triage If there is a suspected stream issue, apply this label to triage if it is something we should fix. Question labels Feb 9, 2023
@robwalch
Copy link
Collaborator

robwalch commented Feb 9, 2023

Is it possible for the npm package installed to have console.assert removed so that the consuming client does not require any special plugin?

All console.assert statements are stripped from hls.min.js. The minified build is the production build. If you are using "hls.js" then you are using the debug build with assert statements included.

@robwalch
Copy link
Collaborator

robwalch commented Feb 9, 2023

Duplicates #5143

@robwalch robwalch added answered and removed Needs Triage If there is a suspected stream issue, apply this label to triage if it is something we should fix. labels Feb 9, 2023
@robwalch
Copy link
Collaborator

robwalch commented Feb 9, 2023

@alexlives if you prefer to use hls.js and want others from experiencing the same issue, please submit a PR that removes the config.mode === 'production' guard from preventing assert statements from being stripped out of development builds:

https://github.com/video-dev/hls.js/blob/master/webpack.config.js#L274

@robwalch robwalch added this to the 1.4.0 milestone Feb 10, 2023
@alexlives
Copy link
Author

I don't think we need to compile out the asserts on all builds. It makes sense to only have it done for the production version. I think on our end we need to evaluate how we're consuming the npm package in our project. We seem to only grab the hls.js file and not the hls.min.js

Thank you for the quick reply.

@robwalch
Copy link
Collaborator

robwalch commented Feb 10, 2023

On video-dev, @gkatsev pointed out that "hls.js" is the default package export. We could do better.

The expectation was that either subsequent builds would strip these statements out or that runtimes would not null console.

You shouldn't have to go to these lengths. This is the second report of this kind, so if there aren't any contributions addressing it shortly, I'll get something in before the next minor release.

robwalch added a commit that referenced this issue Feb 18, 2023
robwalch added a commit that referenced this issue Feb 18, 2023
robwalch added a commit that referenced this issue Feb 22, 2023
@robwalch robwalch added the Verify Fixed An unreleased bug fix has been merged and should be verified before closing. label Feb 27, 2023
@robwalch robwalch removed the Verify Fixed An unreleased bug fix has been merged and should be verified before closing. label Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants