IanVS / prettier-plugin-sort-imports

An opinionated but flexible prettier plugin to sort import statements
Apache License 2.0
1.02k stars 25 forks source link

Astro export const prerender statment sort at top #68

Closed pijusz closed 1 year ago

pijusz commented 1 year ago

Is your feature request related to a problem? There is no way to sort some values at the very top. F.e. Astro "export const prerender = true;"

Describe the solution you'd like I want some statements that are 'not-an-import' to be sorted at the very top.

Additional context Astro Example From:

---
import Layout from "~/layouts/Layout.astro";
import Testimonials from "~/components/Testimonials.astro";

import "~/styles/theme.scss";

export const prerender = true;
---
...

Astro Example To:

---
export const prerender = true;

import Layout from "~/layouts/Layout.astro";
import Testimonials from "~/components/Testimonials.astro";

import "~/styles/theme.scss";
---
...
fbartho commented 1 year ago

My gut response that this is out-of-scope for this plugin, we don't look for arbitrary variable AST nodes. This might be something that would be better designed as a dedicated Astro Prettier Plugin, but it might be messy to do inside of this plugin.

IanVS commented 1 year ago

Thanks for the suggestion, @pijusz. However, I agree with @fbartho, I don't think this is the right tool to do what you want. This plugin is only for sorting imports. You'd probably want to make your own prettier or eslint plugin to move that particular export (it could probably be a very simple plugin).

IanVS commented 1 year ago

I guess with the one caveat, if you have this

---
export const prerender = true;

import Layout from "~/layouts/Layout.astro";
import Testimonials from "~/components/Testimonials.astro";

import "~/styles/theme.scss";
---

Is this plugin moving it from the top and turning it into

---
import Layout from "~/layouts/Layout.astro";
import Testimonials from "~/components/Testimonials.astro";

import "~/styles/theme.scss";

export const prerender = true;
---

If so, I would consider that a bug, and we can probably address it to only sort the imports and not move them.

jasikpark commented 1 year ago

i think that's the issue they had..

jasikpark commented 1 year ago

it is nice that the sorter moves all static imports to the top though

jasikpark commented 1 year ago

"constant exports come first" 🤔

IanVS commented 1 year ago

OK I can confirm that we do move the export to the bottom in the example I gave. But it seems strange that Astro would require the export to come at the top, especially given that imports are always processed first before the rest of the file, which is why it's good practice to put imports at the top of the file, to avoid confusion.

I'm not convinced whether this is a bug on our side or desired behavior, will need to investigate further.

IanVS commented 1 year ago

I don't see anything in the Astro docs saying it needs to be at the top of the file: https://docs.astro.build/en/guides/server-side-rendering/#hybrid-rendering...

fbartho commented 1 year ago

Astro is a runtime ecosystem that has conventions JS/TypeScript do not. A dedicated Astro plugin is recommended if you want to enforce Astro formatting recommendations.

Functionally speaking there's no difference. It's a stylistic recommendation from Astro.

IanVS commented 1 year ago

It's a stylistic recommendation from Astro.

I don't see where Astro even recommends it to be at the top. Do you have a link? I looked through their discord as well, and didn't see mention of it being before imports.

fbartho commented 1 year ago

Hah! I don't use astro, I just scanned over the project in my normal research a year or more back.

I definitely should have clarified that I'm interpreting this as a stylistic convention. I just witnessed a bunch of basic hello-world code that had that in common.

I didn't mean to express that I had authoritative information on the topic! Sorry if I caused confusion.

jasikpark commented 1 year ago
image

confirmation from the @natemoo-re himself 😁

this is just an aesthetic choice 👍

natemoo-re commented 1 year ago

Chiming in from the Astro team: I don't think I've ever seen export const prerender = true at the top of the file? At the very least, it's not a requirement (or explicit recommendation) from our side.

My gut response that this is out-of-scope for this plugin, we don't look for arbitrary variable AST nodes. This might be something that would be better designed as a dedicated Astro Prettier Plugin, but it might be messy to do inside of this plugin.

You're definitely welcome to suggest this as an issue on the prettier-plugin-astro repo @pijusz! I'm not sure if we'll end up implementing it since it's just an aesthetic choice, but maybe folks would like the option.