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.

Sign up for GitHub

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

Jump to bottom

Remove support for some older browsers that probably didn’t work anyway #1080

Merged
merged 3 commits into from
Mar 25, 2020
Merged

Remove support for some older browsers that probably didn’t work anyway #1080

merged 3 commits into from
Mar 25, 2020

Conversation

j-f1
Copy link
Member

@j-f1 j-f1 commented Mar 25, 2020

  • Safari < 12 don’t support .flat(), which we use.
  • KaiOS doesn’t support async functions, and runs on a lot of feature phones which I hope we don’t target.
  • Trident-based Edge (12–18) don’t support .flat().
  • Chrome < 70 doesn’t support Grid or .flat()
  • Samsung Internet < 10 and UC Browser < 13 don’t support .flat().

Note: to get the build script to take the new values, you have to rm -rf node_modules/.cache.

It also adds a smaller set of supported browsers for dev mode. I’m happy to revert this if someone on the team is deving on an older browser.

@vercel
Copy link

vercel bot commented Mar 25, 2020 •
edited
Loading

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/excalidraw-team/excalidraw/qkn1r3goi
✅ Preview: https://excalidraw-git-goodbye-safari-5.excalidraw-team.now.sh

@j-f1
Copy link
Member Author

j-f1 commented Mar 25, 2020

When merging this, we should check the built JS files to make sure the settings have been taken up, and if not, we need to push a commit that does rm node_modules/.cache before building, then revert it.

@lipis
Copy link
Member

lipis commented Mar 25, 2020

Can you run some tests on how much less in bundle size?

@j-f1
Copy link
Member Author

j-f1 commented Mar 25, 2020

The build directory is:

  • 6,140,124 bytes on master
  • 5,938,472 bytes here
    (About 3.3% smaller)

That’s not a lot, but I guess we don’t use async/await or generators a lot, as those two things are really big when compiled to ES5.

@dwelle
Copy link
Member

dwelle commented Mar 25, 2020

Let's maybe add the TS target es6 flag from #942?

@lipis
Copy link
Member

lipis commented Mar 25, 2020

What about now.. does it change anything?

@dwelle
Copy link
Member

dwelle commented Mar 25, 2020 •
edited
Loading

It won't change anything (build-wise). It's just a flag for the TS type checker because CRA doesn't use it for code emit. But it'll make TS server not prevent us from using certain ES6 idioms.

@lipis lipis merged commit 5b9c18a into master Mar 25, 2020
@lipis lipis deleted the goodbye-safari-5 branch March 25, 2020 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwelle dwelle dwelle approved these changes

@lipis lipis lipis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@j-f1 @lipis @dwelle