-
Notifications
You must be signed in to change notification settings - Fork 445
Add hint to enable JavaScript Sources feature in source view error #5643
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5643 +/- ##
=======================================
Coverage 85.76% 85.76%
=======================================
Files 311 311
Lines 30747 30747
Branches 8455 8455
=======================================
Hits 26370 26370
Misses 3953 3953
Partials 424 424 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
locales/en-US/app.ftl
Outdated
| # files when the "JavaScript Sources" feature is not enabled. | ||
| SourceView--no-known-cors-url1 = | ||
| There is no known cross-origin-accessible URL for this file. If this is a | ||
| JavaScript file, you may need to enable the "JavaScript Sources" feature in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| JavaScript file, you may need to enable the "JavaScript Sources" feature in | |
| JavaScript file, you may need to enable the “JavaScript Sources” feature in |
Could we add the linter to this project?
https://github.com/mozilla-l10n/moz-fluent-linter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, forgot that one, fixed it.
Yeah, it would be good to add a linter. Filed #5646 for that.
8e2aac5 to
1cfa84d
Compare
1cfa84d to
10001f7
Compare
|
Hrmm the compromise makes it less useful. It will now also suggest this for C++ functions. Could we instead put the sources table always into the profile, even if the feature is off? And when the front-end requests the source, Firefox could return an error with a message which suggests enabling the feature - I think the existing error handling might just work in that case, no? |
Fixes #5640.
This is not the ideal solution I had in mind initially.
I initially wanted to add a new error, that shows this only when it's a JS file. But when this feature is disabled, there isn't a way to know if this is a JS source. We have
sourceUuid, but it becomes null when this feature is disabled, because we don't serialize the source table in the backend when the this feature doesn't exist (since we don't collect the sources in the first place):profiler/src/utils/fetch-source.ts
Lines 93 to 110 in d396f27
Potentially we can propagate an
isJSargument to this function. But not so sure if that's worth it. I think this is an okay middle ground for this.