-
Notifications
You must be signed in to change notification settings - Fork 123
fix: use same logic to resolve typescript in banner and server #447
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
Conversation
4c2f838
to
20845dc
Compare
@@ -6,65 +6,134 @@ | |||
* found in the LICENSE file at https://angular.io/license | |||
*/ | |||
|
|||
import * as path from 'path'; | |||
const MIN_TS_VERSION = '3.6'; |
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.
shouldn't this be 3.7 since that's going to be the new bundle version?
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.
that's gonna be in the next PR ;) I want to separate the changes from the version upgrade.
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 think a language service will not work with ts 3.7 because the typeArguments
property has been removed from the TypeReference
interface in Typescipt 3.7. Two usages of typeArguments
exist in @angular/.../typescript_symbols.ts file.
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.
Also, please see my comment here #437 (comment)
I think 3.7.2 does not contain the fix
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.
merging pending @andrius-pra approval since this PR doesn't deal with 3.7 yet.
20845dc
to
76358d5
Compare
This commit refactors version_provider.ts to resolve a node module with any minimum version, not just major version. The same logic is now used in server.ts and banner.ts to resolve tsserver. This enables us to accurately report the version of the module require-d.
76358d5
to
4801dd2
Compare
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This commit refactors version_provider.ts to resolve a node module with
any minimum version, not just major version.
The same logic is now used in server.ts and banner.ts to resolve
tsserver. This enables us to accurately report the version of the
module require-d.