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

Check browerlist whether really works #770

Closed
baurine opened this issue Oct 16, 2020 · 9 comments · Fixed by #776
Closed

Check browerlist whether really works #770

baurine opened this issue Oct 16, 2020 · 9 comments · Fixed by #776
Labels

Comments

@baurine
Copy link
Collaborator

baurine commented Oct 16, 2020

Bug Report

Please answer these questions before submitting your issue. Thanks!

What did you do?

Access dashboard in Chrome 5x version.

What did you expect to see?

Work normally.

What did you see instead?

It crashes and says Object.entries() is not a function.

(This version of Chrome is supported in the browser list, so the checkBrowser.js doesn't show a prompt to say this browser version is too low.)

What version of TiDB Dashboard are you using (./tidb-dashboard --version)?

4.0.7

@unbyte
Copy link
Contributor

unbyte commented Oct 18, 2020

I should have fixed it and I'm going to test production bundles under new config. Should I create a PR for it first ?

@baurine
Copy link
Collaborator Author

baurine commented Oct 19, 2020

Yes, please!

@unbyte
Copy link
Contributor

unbyte commented Oct 19, 2020

known:

  • browserslist works well
  • babel injects polyfill well except for Object.entries
  • importing polyfill for Object.entries manually works
  • Object.entries may be in third-party dependencies

@kennytm
Copy link
Contributor

kennytm commented Oct 19, 2020

For record, the stack trace causing the Object.entries error was:

e.exports@http://pd:2379/dashboard/static/js/23.a9c66b1c.chunk.js:1007225
e.exports/</p.onerror@http://pd:2379/dashboard/static/js/23.a9c66b1c.chunk.js:1006176

@baurine
Copy link
Collaborator Author

baurine commented Oct 21, 2020

hi @unbyte , can you help do some more test on Firefox 46 and 47 if it is possible? (not sure whether we still can download such old firefox installation packages). it is reported that has the same error in an old Firefox 4x version (but don't know the exact version).

According to our code logic, it is supposed to show a prompt to ask the user to upgrade their browser in the Firefox below version 68.

Object.entries is supported in Chrome 54+, Edge 14+, Firefox 47+, Safari 10.1+

@unbyte
Copy link
Contributor

unbyte commented Oct 21, 2020

I think it is more appropriate to import polyfill manually as a special case rather than to alert users to change browser

@baurine
Copy link
Collaborator Author

baurine commented Oct 21, 2020

I think it is more appropriate to import polyfill manually as a special case rather than to alert users to change browser

Testing on the old version of firefox is just for checking whether has some more reasons, then we can resolve them together. Importing polyfill manually is one way, or we can higher the lowest supported version if checkBrowser.js works well.

@ti-srebot
Copy link

Please edit this comment to complete the following information

Not a bug

  1. Remove the 'type/bug' label
  2. Add notes to indicate why it is not a bug

Duplicate bug

  1. Add the 'type/duplicate' label
  2. Add the link to the original bug

Bug

Note: Make Sure that 'component', and 'severity' labels are added
Example for how to fill out the template: pingcap/tidb#20100

1. Root Cause Analysis (RCA)

2. Symptom

3. All Trigger Conditions

4. Workaround (optional)

5. Affected versions

6. Fixed versions

@ti-srebot
Copy link

(WorkAroud RCA AllTriggerConditions FixedVersions AffectedVersions Symptom ) fields are empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants