Reworks the build/bundling - #2518
Conversation
503e961 to
85effa0
Compare
Splits the webviews out of the initial bundle to optimize loading perf Replaces moment with day.js to reduce size & improve perf Updates many package dependencies
85effa0 to
abba941
Compare
| Tag | ||
| } | ||
| export { RefType } from './api1'; | ||
| // export const enum RefType { |
There was a problem hiding this comment.
can be removed
| PatchDoesNotApply = 'PatchDoesNotApply' | ||
| } | ||
| export { GitErrorCodes } from './api1'; | ||
| // export const enum GitErrorCodes { |
There was a problem hiding this comment.
also removable
| return [ | ||
| uri, | ||
| { | ||
| hostname: (await HostHelper.getApiHost(hostUri)).authority, |
There was a problem hiding this comment.
uri.authority?
There was a problem hiding this comment.
Good call!
|
|
||
| private _iter: IKeyIterator; | ||
| private _root: TernarySearchTreeNode<E> | undefined; | ||
| static forPaths<E>(): TernarySearchTree<string, E> { |
There was a problem hiding this comment.
several unused things here - forPaths, forStrings, forConfigKeys. should we wait to introduce these utils until they're actually needed?
There was a problem hiding this comment.
Up to you -- I just grabbed the entire thing from the vscode repo (to maybe easier to keep it "in sync")
There was a problem hiding this comment.
that's a fair point, let's leave it as is
| </head> | ||
| <body> | ||
| <div id="app"></div> | ||
| <script nonce="${nonce}" src="${this._webview!.asWebviewUri(uri).toString()}"></script> |
There was a problem hiding this comment.
this also works in the webworker case?
| import { sep } from 'path'; | ||
| import moment = require('moment'); | ||
| import { Disposable, Event, Uri } from 'vscode'; | ||
| import dayjs from 'dayjs'; |
There was a problem hiding this comment.
nice 👍
| } | ||
|
|
||
| return new PullRequestModel(this._telemetry, this, this.remote, convertRESTPullRequestToRawPullRequest(pullRequest, this)); | ||
| return new PullRequestModel(this._telemetry, this, this.remote, convertRESTPullRequestToRawPullRequest(pullRequest as any, this)); |
There was a problem hiding this comment.
I think this cast to any is unneeded
There was a problem hiding this comment.
Yup, not needed anymore
| if (reviewManager) { | ||
| return pushAndCreatePR(currentIssue.manager, reviewManager, this._stateManager); | ||
| } | ||
| break; |
There was a problem hiding this comment.
nice
| repositories_url: { default: 'https://api.github.com/teams/1/repos' }, | ||
| parent: { default: { test: 'test' } }, | ||
| html_url: { default: 'https://api.github.com/teams/1' } | ||
| // parent: { default: { test: 'test' } }, |
There was a problem hiding this comment.
removable
| } | ||
|
|
||
| export async function activate(context: vscode.ExtensionContext): Promise<GitApiImpl> { | ||
| extensionId = 'github.vscode-pull-request-github'; |
There was a problem hiding this comment.
there is actually EXTENSION_ID used below which should capture this and is changed in the build, it could just be marked as not const to be reused here
There was a problem hiding this comment.
Great, thanks!
Rachel Macfarlane (RMacfarlane)
left a comment
Rachel Macfarlane (RMacfarlane)
left a comment
There was a problem hiding this comment.
Looks good!
2 participants
Slims the initial bundle size
Splits the webviews out of the initial bundle to optimize loading perf
Replaces TSLint with ESLint
Replaces moment with day.js to reduce size & improve perf
Updates many package dependencies
Adds esbuild option (on by default)