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

chore: update tsc target #80050

Merged
merged 1 commit into from Aug 30, 2019

Conversation

@LeuisKen
Copy link
Contributor

commented Aug 29, 2019

Update tsc target to "es2017" since vscode now using electorn 4.2.7
which based on chromium@68 and node@10.11.0.

This will enable more esnext features and reduce the bundle size.

According to: https://kangax.github.io/compat-table/es2016plus/

@jrieken jrieken added this to the September 2019 milestone Aug 29, 2019
src/tsconfig.base.json Outdated Show resolved Hide resolved
@jrieken

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

Ran two builds to compare file sizes of our main bundle: before 6027569bytes, after 5994648bytes. That's less what I have expected but the awesome news is that async/await is now just as-is, no more yield, no more state machine.

@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

Why not go for ES2018, we actually seem to be using that already for all of our extensions, even though some seem to explicitly set a target:

"target": "es2018",

I think:

  • core should move to ES2018
  • every extension should import the shared one
  • no extension should overwrite the target
@LeuisKen

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

I agree with @bpasero 's views.

LeuisKen added a commit to LeuisKen/vscode that referenced this pull request Aug 30, 2019
- update tsc target to "es2018" according to microsoft#80050
- update "extensions/markdown-language-features/preview-src"
tsconfig.json using shared.tsconfig.json according to microsoft#80050

Change-Id: If87caed5f6d61a3a110b80ec9d5a63e3421a0693
@LeuisKen LeuisKen force-pushed the LeuisKen:master branch from 48acc4b to de567d4 Aug 30, 2019
@jrieken

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

Using es2018 yields in 5992860bytes but it seems that dev tools fails to pretty print the minified file, e.g you must use source maps which isn't an option for everyone.

@LeuisKen

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

it seems that dev tools fails to pretty print the minified file, e.g you must use source maps which isn't an option for everyone.

Yes, I also reproduce this problem on Chrome 76, so we can't fix this problem by using latest electron...
So what to do with this problem, shall I revert the "target" to "es2017" ?

BTW: I find that the JavaScriptFormatter in Chrome Dev Tool has not been updated since 2018, maybe I could try to make a patch to handle our case.

src/tsconfig.json Outdated Show resolved Hide resolved
- update tsc target to "es2017" according to #8005

Change-Id: Id39241d89ccb92c25a75d89addaf1528cc79ec59
@LeuisKen LeuisKen force-pushed the LeuisKen:master branch from de567d4 to aed1ad4 Aug 30, 2019
@jrieken jrieken merged commit 502fb0f into microsoft:master Aug 30, 2019
1 check passed
1 check passed
VS Code #20190830.34 succeeded
Details
@bpasero

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

@jrieken

Using es2018 yields in 5992860bytes but it seems that dev tools fails to pretty print the minified file, e.g you must use source maps which isn't an option for everyone.

To clarify: using ES2018 results in issues when looking at minified code? But we always ship with sourcemaps no?

@jrieken

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Well, when there is the need to debug minified code (because the issue is so obscure) then I don't trust source maps (they have failed me soo many times) and I want JS code.

@bpasero

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Yeah I saw this issue in action, weird. Even reproduces with latest E6 builds. Looks like a bug in DevTools to me.

@jrieken

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Yeah, sure an issue with dev tools. We can revisit this when we have a newer version or when we swap the minifier.

@LeuisKen

This comment has been minimized.

Copy link
Contributor Author

commented Sep 2, 2019

@bpasero

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Thanks for doing that ❤️

@WSLUser

This comment has been minimized.

Copy link

commented Oct 11, 2019

you can switch to 2018 now. The bug says it's been fixed and has changed to using 2018.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.