-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
feat(cli): --ext parameter for run, compile, and bundle #17172
Conversation
…ment-language-mode
…ment-language-mode
Only if it increases my closed issue count. |
Thanks and sorry, this was done in error. I meant to close #17112, when I copy and pasted the issue number gh did some auto complete weirdness that must have selected this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve conflicts.
For what it's worth, the changes themselves seem fine to me. Consistency is good, and the breaking change here is very minor since it only affects pipe operation and extensionless input files. As such, I'd be willing to "look the other way" in terms of breaking semver. I do not decide that though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks a lot better! Thanks! I have just a few comments.
…ment-language-mode
…ment-language-mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @Cre3per!
I had a look at your commits, looks good. Thanks for your feedback @dsherret! |
@Cre3per and sorry for my delay on this one! I did a few commits just now because I want to get this in the release (happening in a little bit). |
Now $ echo "console.log(1 as any)" | deno run -
error: The module's source code could not be parsed: Expected ',', got 'as' at file:https:///Users/kt3k/denoland/deno_std/$deno$stdin:1:15
console.log(1 as any)
~~
$ echo "console.log(1 as any)" | deno run --ext ts -
1
$ deno -V
deno 1.32.0 Is this expected? (Some test cases in std failed by this change) |
Bug reported here shortly after merging `--ext` changes #17172 (comment) Also found a missing `--check` in integration tests for `--ext` that would have missed a bug if there was one. Fixes #18392
Adds
--ext
todeno run
, closes #5088Additionally
--ext
todeno compile
anddeno bundle
Deprecatesdeno eval --ts
(Was already marked for removal in code, this PR exposes the deprecation to the CLI)deno fmt --ext
fromts
tojs
for consistency. If CLI default arguments are part of deno's API, this violates semver and needs to be undone. Personally, I'd prefer TypeScript to be the default for all commands, but JavaScript was chosen here Request: CLI flag to set language mode #5088 (comment). Changing the default value to TypeScript would not avoid the breaking change, because the main branch is inconsistent already (fmt
vseval
).You'll find a new TODO concerning deno_ast in the changes. I intend to address that TODO before this PR is merged, after I get confirmation that the PR is otherwise ok.