Coder Social home page Coder Social logo

Comments (17)

rubiesonthesky avatar rubiesonthesky commented on June 14, 2024

Regarding the open PR, Wouldn’t it be better to change the code understand any alias name instead of hard coding these Page and Layout names? I haven’t seen this convention in Angular ecosystem. I don’t doubt that it exists but I feel like that it isn’t something that is regarded standard way. And prettier would need to start to add all the aliases that people request.

I could be wrong and this is something that Angular is promoting in their style guide. If that it’s the case, then this change makes a lot sense.

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

@rubiesonthesky I have updated the code, now it will support any alias name :)

from prettier.

sosukesuzuki avatar sosukesuzuki commented on June 14, 2024

I am against adding this feature.

But I am not familiar with Angular, so let me ask a question. Is this rule common among people who write Angular? Or is it only needed by you and your team?

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

I am against adding this feature.

@sosukesuzuki This is not an Angular feature and it will not affect the way you write the code, which means you can still use the @Component with prettier.

But I am not familiar with Angular, so let me ask a question. Is this rule common among people who write Angular? Or is it only needed by you and your team?

The common rule, or let's say a good practice among people who write code(doesn't matter which language/technology) is to have a good naming:

"A good name should give you a good idea about what the variable contains or function does. A good name will tell you all there is to know or will tell you enough to know where to look next. It will not let you guess, or wonder. It will not misguide you. A good name is obvious, and expected. It is consistent. Not overly creative. It will not assume context or knowledge that the reader is not likely to have."

This is a great article to read about the importance of naming in programming https://wasp-lang.dev/blog/2023/10/12/on-importance-of-naming-in-programming

As i said before, the @Component naming is so generic, so using an alias will make the code easier to read and understand, you will not need to guess if this is a component or page or layout:

import { Component } from '@anguler/core';

@Component({/*...*/})
export class MyComponent { /*...*/ }

With the current generic @Component naming, you need to read the code to understand what this component is used for :(

import { Component as Page } from '@anguler/core';

@Page({/*...*/})
export class DashboardPage { /*...*/ }

Ah ok, this is the dashboard page, no need to read the decorator template +500 lines or the class properties/methods :)

import { Component as Layout } from '@anguler/core';

@Layout({/*...*/})
export class AdminLayout { /*...*/ }

Ah ok, this is the admin layout, no need to read the decorator template or the class properties/methods :)

from prettier.

fisker avatar fisker commented on June 14, 2024

I suggest use language comment instead.

@Page({
    selector: 'posts-page',
    template: /* HTML */ `
        <h1>My App</h1>
        <app-todo-list></app-todo-list>
    `,
})

from prettier.

sosukesuzuki avatar sosukesuzuki commented on June 14, 2024

Thanks for the reply.

I know that naming things appropriately is good practice. But what I am saying is that implementing a new feature is a trade-off for new complexity.

If this rule is something that is widely used in the Angular community, then I think it is worth accepting the complexity for this feature. But if this rule is just for you and your team, then I don't think it is worth to increase complexity of Prettier.

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

I suggest use language comment instead.

@Page({
    selector: 'posts-page',
    template: /* HTML */ `
        <h1>My App</h1>
        <app-todo-list></app-todo-list>
    `,
})

Instead of what ?

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

If this rule is something that is widely used in the Angular community, then I think it is worth accepting the complexity for this feature

@sosukesuzuki This is not a rule or an Angular feature, as i said before it will not affect the way you write the code !

Since we both agreed that naming things appropriately is a good practice, then it should/worth be adopted by any good community.

then I don't think it is worth to increase complexity of Prettier.

Don't worry, it will not increase the complexity of Prettier, I have already created a PR to make prettier parse the import from @angular/core and check if @Component is imported then check if it has an alias(local name) or not, + it supports any alias, which make it flexible.

from prettier.

sosukesuzuki avatar sosukesuzuki commented on June 14, 2024

Adding new features necessarily increases complexity. I understand your point, but as a Prettier maintainer I am still against this feature. I recommend using language comments to format your templates, as fisker suggests.

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

Adding new features necessarily increases complexity. I understand your point, I recommend using language comments to format your templates, as fisker suggests.

@sosukesuzuki This is not a new feature, Prettier already supports formatting Angular template string literals, it will just fix/enhance it, a language comment doesn't have to do anything here.

I have already created a PR with tests, so as a maintainer, all you will need is to check/test it.

from prettier.

fisker avatar fisker commented on June 14, 2024

I suggest use language comment instead.

@Page({
    selector: 'posts-page',
    template: /* HTML */ `
        <h1>My App</h1>
        <app-todo-list></app-todo-list>
    `,
})

Instead of what ?

Instead of this PR

Prettier 3.2.5
Playground link

--parser babel

Input:

@AnyNameYouWant({
    selector: 'posts-page',
    template: /* HTML */ `
        <h1>My App
        </h1>
        <app-todo-list></app-todo-list>

    `,

})
class a {}

Output:

@AnyNameYouWant({
  selector: "posts-page",
  template: /* HTML */ `
    <h1>My App</h1>
    <app-todo-list></app-todo-list>
  `,
})
class a {}

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

Instead of this PR

--parser babel

The babel parser doesn't support the new Angular control flow(@if, @for ...)
Playground link.

And even if it supports it, so you suggest/recommend adding an unnecessary comment instead of enhancing Prettier by parsing the Angular import statement ?

from prettier.

rubiesonthesky avatar rubiesonthesky commented on June 14, 2024

You could also just rename your component class names so that they say if they are Page or layout. So instead of DashboardComponent, you have DashboarLayoutComponent. This would be beneficial in other parts of the app too, as it would convey the type when you import those in other components.

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

You could also just rename your component class names so that they say if they are Page or layout. So instead of DashboardComponent, you have DashboarLayoutComponent.

Why you will use the Component suffix for page or layout ?

This would be beneficial in other parts of the app too, as it would convey the type when you import those in other components.

AFAIK you don't import those specific "components" in other components, you import/use the page or layout only in the route file, and you don't need to use the Component suffix since you will have the component key, e.g. { path: "/users", component: UsersIndexPage }, or { path: "", component: DefaultLayout }

from prettier.

fisker avatar fisker commented on June 14, 2024

Instead of this PR

--parser babel

The babel parser doesn't support the new Angular control flow(@if, @for ...) Playground link.

The problem is not the js parser, but because we infer the embeded parser as html while it should be angular, even if you use @Component it still don't support @if, we can add support for /* ANGULAR */comment.

And even if it supports it, so you suggest/recommend adding an unnecessary comment instead of enhancing Prettier by parsing the Angular import statement ?

It's not unnecessary, we highly recommend use comments instead of magic names.

from prettier.

flashios09 avatar flashios09 commented on June 14, 2024

It's not unnecessary, we highly recommend use comments instead of magic names.

I have asked ChatGPT "What's the purpose of using/adding a comment in programming ?"

The response:

Comments in programming serve several important purposes:

Code Documentation:

Explanation: Comments explain what the code does, making it easier for others (and yourself) to understand the logic and purpose of the code when revisiting it later.
Usage Instructions: They can provide instructions on how to use a particular piece of code, function, or class.
Clarification:

Complex Code: For particularly complex or non-intuitive sections of code, comments clarify what the code is doing.
Assumptions and Decisions: They can describe the reasoning behind certain decisions, assumptions made, or potential pitfalls.
Temporarily Disable Code:

Debugging and Testing: During debugging or testing, comments can be used to temporarily disable sections of code without deleting them.
Metadata and Annotations:

Version Information: Comments can include version information, authorship, and date of creation/modification.
To-Do Notes: They can serve as placeholders for future improvements or to highlight areas that need attention.
Readability and Maintainability:

Organizing Code: By using comments to separate different sections, the code becomes more organized and readable.
Maintenance: Comments assist in maintaining the code, especially in larger projects with multiple developers.


As you can see, your recommendation doesn't make any sense, we don't use comments for that, adding /* ANGULAR */ comment before the template literal string in an Angular component, in an Angular app/context, it's unnecessary comment, anyone who will read the code and isn't aware about this issue, will say "WTF this comment is doing here", so we will need to add a comment for this comment to explain why we "need" it.

So let's recap :)

The issue:
Prettier ignore the Angular import statement and use the hard string @Component instead.

How to fix this issue ?
Make Prettier parse the import statement(just 10 lines to add using the AST), check if the Component is imported and if it has a local name(alias).

The "maintainers" response after +20 comments with zero argument:
We will not fix the issue because we just don't want to, we will not even take a look at the PR, just add an unnecessary comment instead to make it "uglier".

from prettier.

fisker avatar fisker commented on June 14, 2024

The decision already made by the team long time ago, the embedded language format feature already frozen.

The language comment I offer is the only possible solution that may
consider to accept by the team. Check "import specifier check" solution is off the table.

Follow this comment #12139 (comment) if you are interested. There are similar PRs blocked/closed even they are send by team members.

from prettier.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.