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

build: add typescript project references #4055

Merged
merged 22 commits into from
May 3, 2022

Conversation

mohanraj-r
Copy link
Contributor

@mohanraj-r mohanraj-r commented Apr 22, 2022

What does this PR do?

  • Make use of Typescript project references to declare internal mono-repo package dependencies
    • Have a common config at root
    • Packages inherit and override as necessary
  • Inherit from tsconfig/bases
    • Update es target/lib to node14
  • Refactors post compile steps (webpack, copy files) present in certain packages to separate npm scripts so they can be invoked after tsc build
  • Adds separate tsconfig files for ts-loader used to process Typescript files in Webpack as
  • Should result in faster compile
    • Incremental smart compilation with watch mode with a single compile/watch command for the entire project instead of running compile/watch for each individual package which is not optimal
    • Clean compiled output selectively and quickly without having to remove out dirs and recompile all packages

What does this PR doesn't do?

  • Remove 'out' from import paths of internal mono-repo packages (e.g. '@salesforce/salesforcedx-utils-vscode/out/src/cli')
    • Jump to Type defs of TS classes should work (e.g. SfdxCommandBuilder)
  • Update target up from es6 or update vscode version (build: add typescript project references #4055 (comment))

What issues does this PR fix or reference?

@ W-10706475@

Functionality Before

Typescript project config was duplicated per package with no reuse and no references.

Functionality After

Typescript project configs are reused with a common config and project references have been added.

QA Notes

  • Given that this affects how all packages are compiled, perform E2E testing with VSIX files
    • Compare size of VSIX files with develop/main

@mohanraj-r mohanraj-r requested a review from a team as a code owner April 22, 2022 02:47
@mohanraj-r mohanraj-r self-assigned this Apr 22, 2022
package.json Outdated Show resolved Hide resolved
tsconfig.common.json Outdated Show resolved Hide resolved
@mohanraj-r
Copy link
Contributor Author

Failing with various flavours of the following error - Need to debug/fix

Failed to parse file: C:\Users\circleci\project\packages\salesforcedx-vscode-core\out\src\sfdxProject\sfdxPackageDirectories.js

Transformation error; return original code

Error: Line 13: Unexpected identifier
    at constructError (C:\Users\circleci\project\packages\salesforcedx-test-utils-vscode\node_modules\esprima\esprima.js:2407:21)

@mohanraj-r mohanraj-r requested a review from gbockus-sf April 25, 2022 22:40
"noImplicitAny": false,
// Generating Type Definitions from Javascript is blocked by requiring private name
// https://github.com/microsoft/TypeScript/issues/37832
// "allowJs": true, // for the beautify js files
Copy link
Contributor

Choose a reason for hiding this comment

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

would we ever uncomment this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are about a half-dozen failures in a couple of packages due to this check. So yes if we can clean them up we can have this enabled. Another way is to have this only in the packages that are failing and have it enabled globally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a WI and linked

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry misread as question about noUnusedLocals in the tsconfig common json.
For allowJs guess as long as we have the beautify JS src files checked in (not sure if there is a third party lib that can be imported instead that has been created since) and the TSC issue is open. This is in case someone wonders why are the JS files being copied over instead of being run through tsc. Updated comment.

Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

Great work! I didn't QE with changes (well I did a compile before the last set of commits), but this seems like a build task so if it build it builds. If there are any steps required for local dev for this to start working you might call it out in slack after it merges.

@mohanraj-r
Copy link
Contributor Author

@gbockus-sf If/when you have a moment can you please review the recent (mostly doc) changes.

@mohanraj-r
Copy link
Contributor Author

mohanraj-r commented Apr 28, 2022

Extension sizes using https://github.com/lcampos/circleci-to-vscode

$ du -sh ~/.vscode-insiders/extensions/
491M    

$ du -sh ~/.vscode-insiders/extensions/salesforce.*
 88K    salesforce.salesforcedx-vscode-54.10.0
 57M    salesforce.salesforcedx-vscode-apex-54.10.0
5.3M    salesforce.salesforcedx-vscode-apex-debugger-54.10.0
 34M    salesforce.salesforcedx-vscode-apex-replay-debugger-54.10.0
153M    salesforce.salesforcedx-vscode-core-54.10.0
 28K    salesforce.salesforcedx-vscode-expanded-54.10.0
 42M    salesforce.salesforcedx-vscode-lightning-54.10.0
159M    salesforce.salesforcedx-vscode-lwc-54.10.0
 24M    salesforce.salesforcedx-vscode-soql-54.10.0
 18M    salesforce.salesforcedx-vscode-visualforce-54.10.0

Comparing with VSIX files from a recent build on develop it can be noticed that the size hasn't changed significantly

$ du -sh ~/.vscode-insiders/extensions/
485M    

$ du -sh ~/.vscode-insiders/extensions/salesforce.*
 88K    salesforce.salesforcedx-vscode-54.10.0
 56M    salesforce.salesforcedx-vscode-apex-54.10.0
4.7M    salesforce.salesforcedx-vscode-apex-debugger-54.10.0
 33M    salesforce.salesforcedx-vscode-apex-replay-debugger-54.10.0
150M    salesforce.salesforcedx-vscode-core-54.10.0
 28K    salesforce.salesforcedx-vscode-expanded-54.10.0
 41M    salesforce.salesforcedx-vscode-lightning-54.10.0
159M    salesforce.salesforcedx-vscode-lwc-54.10.0
 24M    salesforce.salesforcedx-vscode-soql-54.10.0
 18M    salesforce.salesforcedx-vscode-visualforce-54.10.0

Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

:shipit: Great work

@mohanraj-r mohanraj-r merged commit 264ab2f into develop May 3, 2022
@mohanraj-r mohanraj-r deleted the mohanraj-r/typescript-project-ref-enable branch May 3, 2022 18:15
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.

2 participants