-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[Storage] Use ES2017 #15835
[Storage] Use ES2017 #15835
Conversation
With this change, are all Track 2 packages in our repo now targeting ES2017? |
@ramya-rao-a yes all of them target ES2017. @xirzec already updated the root tsconfig file. I have it explicit in some packages because for some reason they do not extend the root tsconfig and I am not sure why. |
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.
I'm good with this change, but I think we need to bump some versions as part of it
// TypeScript imports should use src directly | ||
relativePath += "/src"; | ||
} | ||
|
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.
This was done before for reasons that @witemple-msft does not remember and it is causing the storage-file-datalake samples to fail compiling because storage-common is just a simple folder of source files instead of being a package with a package.json
(unlike keyvault-common) so compiled files there do not have access to tslib.
@@ -2,7 +2,6 @@ | |||
"extends": "../../../tsconfig.package", | |||
"compilerOptions": { | |||
"strict": true, | |||
"target": "es5", |
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.
this was needed because it depends on core-http. There is no need to bump the version I believe since it is still in beta.
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.
I'm good with this change, but we should make sure to merge this and #15925 at the same time.
Also we should get the Storage team to sign off before merging.
Update: I have pushed a few commits to keep the version updates of the core packages out of this PR. |
@EmmaZhu, @XiaoningLiu, @ljian3377, We are going ahead with this change to standardize the target in tsconfig file as we have other PRs waiting on this. Please use #15671 to discuss any concerns you have here. We are doing a minor version update as this is not a bug fix. |
Fixes #15640
Fixes #15671
Updates core-http, core-lro, and storage packages to compile to ES2017.