Skip to content

[api] Add createIncrementalProgram - #64401

Draft
Andrew Branch (andrewbranch) wants to merge 14 commits into
microsoft:mainfrom
andrewbranch:agents/explore-createincrementalprogram-js-api
Draft

Andrew Branch (andrewbranch) wants to merge 14 commits into
microsoft:mainfrom
andrewbranch:agents/explore-createincrementalprogram-js-api

Conversation

@andrewbranch

Copy link
Copy Markdown
Member

Part of #63875

This is kind of an interesting one. In an incremental program, emitting is a state-changing operation, so this is modeled as a snapshot change:

using program = api.createIncrementalProgram(rootFiles, options);
const { snapshot, program, ...emitResult } = program.emit();
// returns new snapshot and incremental program along with the emit result
snapshot.dispose();

This was mostly Copilot with a few high-level conceptual corrections from me; implementation still needs a closer review from me.

Restore persistent diagnostic and emit state from tsbuildinfo while keeping checker and language-service operations backed by the underlying compiler program.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoid exposing the shared program ownership helper as a generated API method and construct fresh compiler options in tests to satisfy no-copy checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This comment was marked as resolved.

# Conflicts:
#	packages/typescript/src/api/async/api.ts
#	packages/typescript/src/api/proto.generated.ts
#	packages/typescript/src/api/sync/api.ts
#	tsc/internal/project/projectcollectionbuilder.go
#	tsc/internal/project/snapshot.go

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This comment was marked as resolved.

@dragomirtitian

Copy link
Copy Markdown
Contributor

Trying to integrate this in our build tool. One small issue, createIncrementalProgram does not take a VFS. I worked around this by using createSnapshot directly, but createIncrementalProgram call setOwnedSnapshot on the program, which is internal, so probably this isn't a good way to o it long term.

@andrewbranch

Copy link
Copy Markdown
Member Author

Can you elaborate on the issue? That sounds right to me. A VFS isn't a property of a program, it's something you can apply to a snapshot, and then any programs of any kind you add to that snapshot will use the VFS. api.createIncrementalProgram is just a convenience method to be used when you don't need to interact with the snapshot directly. The setOwnedProgram path makes it so you can dispose the snapshot by disposing the program, since we know from api.createIncrementalProgram or api.createProgram that there's exactly one Program in the Snapshot and we never gave you a direct handle to the Snapshot itself, so you need a way to dispose it. If you use createSnapshot or snapshot.update, you should dispose the Snapshot directly.

@andrewbranch

Copy link
Copy Markdown
Member Author

I guess you could argue that since using an incremental program usually involves changing its state by emitting, the convenience method that gives you the program directly without the snapshot makes it kind of confusing... you really need to know how to manage a snapshot if you're going to use an incremental program.

This comment was marked as resolved.

This comment was marked as resolved.

@dragomirtitian

Copy link
Copy Markdown
Contributor

I guess you could argue that since using an incremental program usually involves changing its state by emitting, the convenience method that gives you the program directly without the snapshot makes it kind of confusing... you really need to know how to manage a snapshot if you're going to use an incremental program.

If we can manage the snapshot ourselves then that is fine I guess. It's juts that the convenience method is of no use to us, but it might be of use to others who don't need to apply a VFS on the snapshot.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

@dragomirtitian

Copy link
Copy Markdown
Contributor

I tested our build tool with the changes in this PR. Shape of the API works for us. Everything seems to work.

@andrewbranch

Copy link
Copy Markdown
Member Author

Great! Do you see performance improvements over using non-incremental programs?

# Conflicts:
#	packages/typescript/src/api/async/api.ts
#	packages/typescript/src/api/sync/api.ts
#	packages/typescript/test/sync/api-generators.test.ts
#	tsc/internal/api/proto.go
#	tsc/internal/api/session.go

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants