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

[TypeScript] Standardize on .js suffix #3838

Merged
merged 1 commit into from
Sep 3, 2023

Conversation

Thom1729
Copy link
Collaborator

@Thom1729 Thom1729 commented Sep 3, 2023

The TypeScript syntax uses the .js suffix, but there were a couple of places where a .ts slipped through.

Also strip the .js suffix from the tests, as is convention.

@keith-hall
Copy link
Collaborator

The TypeScript syntax uses the .js suffix,

Why? JS != TS... Normally it makes sense when inheriting syntaxes use a separate suffix which would match their base scope suffix, then it's clear whether a token is from the original syntax or the extended one etc.

@deathaxe
Copy link
Collaborator

deathaxe commented Sep 3, 2023

Actually TypeScript, JSX and TSX have followed the route of using .js suffix from beginning on. Hence this PR is just consequently maintaining scope naming consistency.

It doesn't probably make sense to enforce unique suffixes as it may quickly end up looking like wild mess, especially if brackets or other enclosing punctuations/tokens are re-used from between syntaxes. In such cases it's just better to decide for a generic suffix and re-use it everywhere.

#3575 may be a good example for wildly mixed brace scopes, sometimes even not balanced.

Hence distinct suffixes may make sense only, if completely unique features (statements or expressions) are to be emphasized. They should not be used just to find the source of a pattern.

@Thom1729
Copy link
Collaborator Author

Thom1729 commented Sep 3, 2023

↑ What @deathaxe said.

In principle, I think that we could identify TS-specific tokens and give them the .ts suffix. In practice, I think that this would be annoying to maintain and not very useful.

@jrappen
Copy link
Contributor

jrappen commented Sep 3, 2023

Keith, you can still check against base scopes (i.e. source.js vs. source.ts).

@Thom1729 Thom1729 merged commit 361a98a into sublimehq:master Sep 3, 2023
@Thom1729 Thom1729 deleted the typescript-replace-ts-with-js branch September 3, 2023 18:10
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.

4 participants