Investigate TypeScript 5 compatibility
simonhaenisch opened this issue · 26 comments
Lots of new options that potentially need to be exposed.
We have 40+ repos with failing prettier checks since TypeScript v5. Up until then everything was fine (prettier +
prettier-plugin-organize-imports), so any "fix" to restore the former behaviour would be great.
Ok cool do you have any details with how they are failing exactly? Can they be auto-fixed? It might just be that it failed because the default import sort order changed?
Ok cool do you have any details with how they are failing exactly? Can they be auto-fixed? It might just be that it failed because the default import sort order changed?
Yes, the changed sort order is the problem.
Instead of import {isExternalState, isJobState, PortingOrderJob} from... prettier now expects import {PortingOrderJob, isExternalState, isJobState} from.... This can be fixed by running npx prettier -w . - but we do not want to do this, because a lot of repos are affected. When saving a file in VS Code, the new sort order is not applied; the file stays unchanged, which is not really what I expected, since the "organizeImport" stuff works great otherwise. Maybe a new version of VS Code is needed...
Ok I see, I think it depends on whether you have VS Code set up to use the workspace's TypeScript version or the built-in one. See this guide for changing it: https://code.visualstudio.com/docs/typescript/typescript-compiling#_using-newer-typescript-versions.
Anyway, so you'd rather want an option here that allows you to go back to the old sorting behavior? I'll have to check first how these options need to be passed, not sure I'll have any time for it this week tbh.
Thumbs down for an option to configure sort order, for the same reason that Prettier itself doesn't have these kinds of options. This plugin (and the ecosystem more generally) should just use the default TypeScript values, whatever those are, in the same way that the ecosystem should use the default Prettier values, whatever those are.
we do not want to do this, because a lot of repos are affected
Prettier --write should be able to fix everything automatically, even if a lot of repos are affected. Shouldn't we expect breaking changes like this when we upgrade our monorepos to TypeScript to 5.0? That does not strike me as a valid reason for requesting a new option from the plugin.
Shouldn't we expect breaking changes like this when we upgrade our monorepos to TypeScript to 5.0?
TypeScript version numbers do not follow SemVer.
That does not strike me as a valid reason for requesting a new option from the plugin.
I'm not requesting anything; just exploring the possibility to keep the former behaviour.
TypeScript version numbers do not follow SemVer.
Indeed. But the consequence of that is you should NEVER update your TypeScript version (even if it is just a new patch version) unless you want breaking changes in your codebase.
I think the more salient point is that naturally, if one choses to update their TypeScript dep, they will generally expect new breaking type-related errors from the compiler. But they would not necessarily expect new breaking formatting changes from Prettier, since Prettier is an altogether different tool from TypeScript. So it is reasonable to be surprised when this turns out to be the case.
With that said, I would still argue that we should not be allowed to configure anything. Instead, we should expect formatting errors when we upgrade our TypeScript dep (even if it is counterintuitive), and if that is undesired, then we should not update our TypeScript dep. (Essentially, we should treat formatting-errors in the same way we treat type-errors.)
Kinda agree with @Zamiell's general stance on this. Whether TS follows semver or not doesn't really matter here, but also they kinda waited with all these changes for a new major version, so in that regard they actually did follow semver.
The plugin is very clear about using the TypeScript Language Service API, and defines typescript as a peer dependency, so I agree it should be somewhat expected that updating to a new TS version (especially major) can contain changes to organizeImports in the service language API that would affect the output of this plugin (it's not the first time, in TS 4.7 they introduced respecting empty lines between import statements as a separator for grouping imports).
I also agree that it shouldn't be a great problem to re-run prettier along with the PR that upgrades to TypeScript 5, might just require some minor rebasing of conflicts in some PRs.
All that being said, I wouldn't mind exposing the options that TypeScript offers. I only want to make sure they are actually stable so I don't need to end up adding/removing options all the time. Reading about it sounds more like those options are still experimental. If I could just expose the whole options object that would be ideal but I don't think prettier officially supports objects as options (just booleans, numbers and strings).
I've the same problem as @maplesteve but with IntelliJ. For me it's kinda weird. I can fix the issues running prettier --write, but i've also configured in IntelliJ that prettier runs as a save action. And when changing a file, prettier runs (and the plugin) and changes the import order statements back to the previous order so that prettier checks are failing again. Example
Original file contents:
export * from "./types";
export * from "./UrlParams";
export * from "./useUrlParams";prettier --check complains
Contents after prettier --write
export * from "./UrlParams";
export * from "./types";
export * from "./useUrlParams";prettier --check passes
Now I add a space to the end of the file for example and trigger a reformat (and reorganize).
File contents:
export * from "./types";
export * from "./UrlParams";
export * from "./useUrlParams";Optimize imports in IntelliJ as a save action is turned off, only prettier runs. Typescript is used from projects node_modules. Any idea how this could happen?
Hey have you tried restarting IntelliJ? Maybe it was using a cached TS version from before you changed it to use the project TS?
Yes, having this problem since the release because I tried to update to v5 on the first day. Now I'm still on 4.9.x
I'm having similar problems using prettier in vs code with the prettier extension - after upgrading typescript this plugin stops working in vs code. Running prettier in the cmd line works fine. Both vs code's internal typescript and the locally installed typescript are set to 5.0.2.
Could you maybe search or open a discussion or issue in the Prettier VS Code/IntelliJ extension repo, to ask why the extension wouldn't run the TS prettier version of the project?
Also, can you try setting prettier.prettierPath to the local project path?
Actually I just remembered this #84 (comment):
I have no idea why the Prettier extension runs prettier from the root directory and not the current VS Code project root
I feel like this could be causing this problem because it might not be able to resolve the project's node_modules, i.e. local TS version? Don't have time to investigate this currently though. Maybe try whether you can get it fixed with the prettierPath plugin config option.
In my IntelliJ, typescript and prettier are used from the projects node_modules folder as per settings in the UI.
Ok, could you please try patching node_modules/prettier-plugin-organize-imports/index.js, there's a block
if (process.env.DEBUG) {
console.error(error);
}and comment out the if-block so it always logs the error, and then check your IntelliJ plugin logs if you can see anything? Feel free to add extra console.logs, e.g. to make sure the plugin definitely is executed.
I messed around with the prettier plugin settings in vs code and suddenly this plugin works again! It started working specifically after I unchecked the "Use Editor Config" setting. After checking it again it keeps on working though
Strangely I cannot get any debug logs from the organize imports plugin through the vs code Prettier output window though. The logs from prettier itself seem to indicate that the correct version is run (the one installed in the local project's node_modules).
Thats very odd... I disabled the EditorConfig support in IntelliJ like @DennisRosen in VS Code and voilá, my problem was fixed. After enabling the setting again it still worked
I think in that case it might be a bug with the editor config option in the implementation of those IDE plugins then, or more likely a bug in Prettier itself it both (or other) IDE plugins rely on some shared code from the prettier could base.
If you think it's worth investigating, you could open an issue in https://github.com/prettier/prettier or https://github.com/prettier/prettier-vscode. But probably only worth it if you can reproduce this by downgrading the TS version to latest 4.x and toggling the same option again, and after that upgrade back to TS 5.x.
Just to be curious, can you "just" up the typescript devDependency to v5+ and see if this changes anything?
I've been using the plugin in some projects that I upgraded to TS 5 recently, and it was working just fine (except re-ordering a few imports). The line you linked is from dev dependencies (just used during plugin development/testing). Once you install the plugin into your project, it uses the typescript of your project (just has to match what's requested via peer dependencies).
So bumping the version in dev dependencies doesn't change anything except running the tests with TS 5 (but I'm pretty confident they would pass). Feel free to open a PR if you want to try ;) I still need to set up Dependabot for this repo
except running the tests with TS 5
my comment aimed explicitly for that
I know how devDependencies are working
I tried to add a test case, but can not reproduce it and reported the bug upstream to TypeScript itself
Not sure if it is of any help, but we fixed it by pointing VSCode explicitly toward the installed prettier version:
// .vscode/settings.json
{
// some other vscode config stuff
"prettier.prettierPath": "./node_modules/prettier"
}
Intellij kept working fine.
Closing this now as it seems to work fine with TypeScript 5 from what I can tell.