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

Feature Request: Targeting Only Subdirectories in a Monorepo #159

Closed
FrancesCoronel opened this issue Mar 15, 2024 · 8 comments · Fixed by #161
Closed

Feature Request: Targeting Only Subdirectories in a Monorepo #159

FrancesCoronel opened this issue Mar 15, 2024 · 8 comments · Fixed by #161
Assignees

Comments

@FrancesCoronel
Copy link

FrancesCoronel commented Mar 15, 2024

Is your feature request related to a problem?

I want to introduce a standardized import order at my company. After some research, this seems like the best plugin to do that as it does not re-order across side-effect imports and is more actively maintained than the Trivago plugin. 🎉

After some discussion with our FE infra team, they're open to incrementally applying this standard to specific subdirectories until we eventually get to the root.

However, it doesn't look like targeting subdirectories is currently possible with this plugin.

Describe the solution you'd like

I want to use Prettier's built-in configuration overrides method to apply import sorting only to specific subdirectories.

https://prettier.io/docs/en/configuration.html#configuration-overrides

Here is an example of how I would update the root .prettierrc file to target certain directories.

Note that I introduced `"importOrder": null" as a way to not require sorting imports for all files except the ones defined in overrides.

{
	"printWidth": 100,
	"tabWidth": 4,
	"useTabs": true,
	"singleQuote": true,
	"importOrder": null,
	"overrides": [
		{
			"files": [
				"js/modern/components/WIP/feature/**/*.{ts,tsx,js,jsx}",
				"js/modern/redux/stores/feature-store/**/*.{ts,tsx,js,jsx}"
			],
			"options": {
				"importOrder": [
					"<THIRD_PARTY_MODULES>",
					"",
					"^react$",
					"",
					"^@feature/(?!.*\\.(less|css)$)(.*)$",
					"",
					"^@root/img/(.*)$",
					"",
					"^[./]",
					"",
					".*\\.(less|css)$"
				]
			}
		}
	]
}

Describe alternatives you've considered

The only alternative would be to format the entire monorepo simultaneously, which would be very risky given the size of the diff and because we are targeting imports which can lead to run time errors if incorrectly updated.

Additional context

Thank you for all your work on this! 💯

@IanVS
Copy link
Owner

IanVS commented Mar 15, 2024

@FrancesCoronel thanks for the feature request! I did a little bit of experimenting and digging around, and found that Prettier v4 (alpha) does not support overrides for plugin options at all right now (see prettier/prettier-cli#9). So if you're using that version, then unfortunately we're out of luck until that is resolved.

I also tested out my app using Prettier v3, and I found that I was indeed able to change the importOrder inside of overrides, so at least that part is working. But, there's no way to disable the plugin at the base config (hence your proposal of importOrder: null. Am I understanding that correctly? Are you able to change the config using overrides successfully, but not turn off the plugin everywhere else?

@FrancesCoronel
Copy link
Author

Oh yeah, we are on Prettier v3.2.5, so thankfully, all good there.

But yes, you understand that correctly! 💯

Thousands of linting errors are still produced with the example I shared earlier since the importOrder provided out of the box still gets applied to the rest of the codebase, even if you don't actively define it anywhere in the Prettier config.

There's not currently a way to turn off the plugin elsewhere, hence the proposal to add a special phrase to override the out-of-the-box behavior while still being able to target subdirectories. 💡

@IanVS
Copy link
Owner

IanVS commented Mar 15, 2024

@FrancesCoronel I pushed up a PR that allows using importOrder: [] to disable the plugin. It worked as expected when I tried it out in a project of mine, allowing me to turn off the plugin by default and then set the sort order in individual folders / globs. We should be able to release a new version with that functionality soon.

@FrancesCoronel
Copy link
Author

Awesome!! 🤩

This is going to make our incremental adoption so much easier - thank you!

Is there a way to donate to you for your efforts? GitHub Sponsors maybe?

@IanVS
Copy link
Owner

IanVS commented Mar 15, 2024

Thanks, that's kind of you to offer. I do have GitHub sponsors set up if you'd like to use that, but of course you shouldn't feel under any obligation. This request made sense to have, and I'm glad to help out when I can.

IanVS added a commit that referenced this issue Mar 17, 2024
Closes #159

This changes up how we handle `importOrder: []` in the prettier config.
Previously, we would have applied our default config, such that
builtins, node modules, and relative imports would be sorted. Now, it
effectively disables the plugin so that no sorting takes place.

This could be seen as a breaking change, but instead I see it as a bug
fix. It doesn't make sense that using an explicit empty array for the
setting causes a fallback to be used.

Additionally, this gives users more control over disabling the plugin
and turning it on only for certain subfolders or types of files, as
shown in the referenced issue and documented in the readme in this PR.
@IanVS
Copy link
Owner

IanVS commented Mar 18, 2024

Released in 4.2.1. Let me know if you hit any issues!

@IanVS
Copy link
Owner

IanVS commented Jun 11, 2024

@FrancesCoronel just curious, how is the rollout going? Anything else that would be helpful?

@FrancesCoronel
Copy link
Author

FrancesCoronel commented Jul 11, 2024

@IanVS Everything has been going great! 💯

I'm trying to get to 100% adoption, but we currently have about 300 directories added. I just posted a reminder to folks yesterday to add their team's directories if they haven't already.

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 a pull request may close this issue.

2 participants