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

Go full typescript #1333

Closed
wants to merge 1 commit into from
Closed

Go full typescript #1333

wants to merge 1 commit into from

Conversation

kumaraditya303
Copy link

No description provided.

@kumaraditya303 kumaraditya303 marked this pull request as draft January 1, 2021 12:23
plugins: [
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
new TsconfigPathsPlugin({
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erictraut
Copy link
Collaborator

The existing build scripts are working fine for us and are designed to work with the build scripts for pylance, which is a private repo. It's not clear to me what benefit we get from these changes. It would have to be significant to offset the risk of regressions. In any case, I'm not in a position to review or approve this change because @jakebailey is responsible for the existing scripts, so we'll need to wait for him to weigh in when he returns from the holidays.

Signed-off-by: Kumar Aditya <[email protected]>
@kumaraditya303 kumaraditya303 marked this pull request as ready for review January 2, 2021 04:50
@jakebailey
Copy link
Member

Yeah, I don't know if this is a good idea quite yet; I'll have to ensure that this also happens for Pylance and update all of our scripts to be typescript aware, which will take some effort, plus dealing with the troubles of our nested project layout when it comes to TS configs.

This PR also adds in the old webpack 4 types, which aren't entirely accurate given we're on webpack 5. I'm not sure if that's really a regression, though; I think there are some oddities where TS was already picking up webpack types from the wrong place (even though webpack 5 should be shipping with webpack types).

There are also some package additions that I don't think are needed at all; the top-level workspace package definitely shouldn't need to know about webpack. pyright-internal is gaining more deps when no changes where made, etc.

@erictraut
Copy link
Collaborator

I'm going to close this. If we decide to move forward with parts of it, we can refer back to it then.

@erictraut erictraut closed this Jan 4, 2021
heejaechang pushed a commit to heejaechang/pyright that referenced this pull request Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants