Skip to content

Conversation

@joanlopez
Copy link
Contributor

@joanlopez joanlopez commented Oct 13, 2025

What?

Instead of stripping types (github.com/evanw/esbuild) only when the file extension is .ts, it also does it (or tries to) when the script is provided directly through stdin (instead of file) and it cannot be parsed as JavaScript/ECMAScript source.

Why?

Because currently, if the user provides TS source directly through stdin, k6 throws an error -the one it gets when trying to parse that code as JavaScript/ECMAScript source- and never tries to parse it as TS.

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

Closes #4773 and #5210

@joanlopez joanlopez added this to the v1.4.0 milestone Oct 13, 2025
@joanlopez joanlopez self-assigned this Oct 13, 2025
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 13, 2025 12:55 — with GitHub Actions Inactive
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 13, 2025 13:02 — with GitHub Actions Inactive
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 13, 2025 13:04 — with GitHub Actions Inactive
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 13, 2025 14:11 — with GitHub Actions Inactive
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 13, 2025 14:13 — with GitHub Actions Inactive
@joanlopez joanlopez changed the title Compiler: Support for stdin TypeScript code Compiler: Support for stdin TypeScript source code Oct 16, 2025
@joanlopez joanlopez marked this pull request as ready for review October 16, 2025 09:58
@joanlopez joanlopez requested a review from a team as a code owner October 16, 2025 09:58
@joanlopez joanlopez requested review from codebien, mstoykov and oleiade and removed request for a team October 16, 2025 09:58
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 16, 2025 10:03 — with GitHub Actions Inactive
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 16, 2025 10:05 — with GitHub Actions Inactive
oleiade
oleiade previously approved these changes Oct 20, 2025
return nil, "", err
}

if isTsExtensionFile {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not certain we should report the stdin parsing as well ... if we are going to do it.

I guess we can only report it past the StripTypes if the worry is that this might not work and we will report stuff more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the reason why I left it as-is is because I think it keeps the current behavior, on which we were only reporting usage when the file extension is indeed .ts.

But yeah, I agree that we could move that to once we have stripped types with no error (err == nil), so that way we would report it in both cases. Is that what you mean? Do you also think that would make sense?

If so, I'm happy to add that change here, before merging 👍🏻

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah 👍

mstoykov
mstoykov previously approved these changes Oct 21, 2025
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

LGTM, left a usage related, not blocking comment

@joanlopez joanlopez dismissed stale reviews from mstoykov and oleiade via 7d009f4 October 21, 2025 11:09
@joanlopez joanlopez requested review from mstoykov and oleiade October 21, 2025 11:10
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 21, 2025 11:16 — with GitHub Actions Inactive
@joanlopez joanlopez temporarily deployed to azure-trusted-signing October 21, 2025 11:17 — with GitHub Actions Inactive
Copy link
Contributor

@codebien codebien left a comment

Choose a reason for hiding this comment

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

LGTM if CI is going to be green

@joanlopez joanlopez merged commit d9bea64 into master Oct 21, 2025
39 of 40 checks passed
@joanlopez joanlopez deleted the stdin-ts branch October 21, 2025 13:24
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.

k6 cli missing switch to enable Typescript mode

4 participants