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

Sourcemaps don't work right #3701

Closed
stsvilik opened this issue Oct 21, 2016 · 3 comments
Closed

Sourcemaps don't work right #3701

stsvilik opened this issue Oct 21, 2016 · 3 comments

Comments

@stsvilik
Copy link

stsvilik commented Oct 21, 2016

Description

Sourcemaps provided with videojs do show up in the source tree of a browser, but do not permit setting of a breakpoint in the right spot. When minified file is included, then sourcemaps don't work at all - showing a source that went into a minification, without listing files individually. Perhaps its time to switch to gulp with better sourcemap tools.

Steps to reproduce

Open Chrome Developer Tools

  1. Go to sources tab
  2. Expand source tree provided by video.js script
  3. Set a breakpoint in src/js/video.js somewhere inside videojs function
  4. Refresh page

Results

Expected

Breakpoint to stop exactly where you place it and permit inspection of the code.

Actual

Breakpoints jump all over the code (maps are not in sync with code).

Additional Information

versions

videojs

5.11.8

browsers

Chrome, and any other that support sourcemaps

OSes

MacOS El Capitan

plugins

None

@gkatsev
Copy link
Member

gkatsev commented Oct 24, 2016

They don't. They're already removed in 5.12 and I'll be removing them from 5.11: #3710.
We'll consider re-adding the proper sourcemap support in the future.
Unfortunately, switching to gulp is a non-trivial task will little gain. The issues we are having with sourcemaps won't be magically solved by using gulp instead of grunt.

@gkatsev gkatsev closed this as completed Oct 24, 2016
@stsvilik
Copy link
Author

@gkatsev for basic build, I was able to switch to gulp in a matter of minutes and that helped me create sourcemaps I could use to debug. I think the biggest problems in sourcemaps you provide is that you concat multiple files with their own sourcemaps, instead of generating final combined sourcemap for all concatinated and application code files.

@gkatsev
Copy link
Member

gkatsev commented Oct 24, 2016

It's definitely doable. But the tool ecosystem isn't really the issue. We're definitely doing some weird things with how it's built (mainly with vttjs) but we're going to address that part. Then, we just need to configure the tools we have correctly and make sure that the sourcemaps either get uploaded to the CDN (#3106) or not referenced from the CDN urls. In the meantime, it's more expedient for us to remove sourcemaps altogether because bad sourcemaps provide a much worse user experience than no-sourcemaps.

gkatsev added a commit that referenced this issue Oct 24, 2016
Source Maps, while nice, aren't perfect and the ones we were outputting
weren't right.

Fixes #3701, #3106.
gkatsev added a commit that referenced this issue Oct 25, 2016
Source Maps, while nice, aren't perfect and the ones we were outputting
weren't right.

Fixes #3701, #3106.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants