ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.5k stars 783 forks source link

npm run generate prefixes #2021

Open JohnHardy opened 4 years ago

JohnHardy commented 4 years ago

Stencil version:

@stencil/core@1.8.1

I'm submitting a:

[ ] bug report [x] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

For good reasons I'm sure, the very handy npm run generate tool has changed to remove the prefix from components (https://github.com/ionic-team/stencil/pull/1992).

However, for projects that already use this tool (and have 100s of components and prefixes everywhere) it means quite a bit of extra faff to go through and manually rename everything.

Would you consider a command switch --keep-prefix to use the full name of the component?

Or if there is a way to clone and adapt the tool and keep in my project, please could somebody point me to the relevant documentation?

image 😄

brettchalupa commented 4 years ago

I ran into this today when going through the tutorial and trying out the generator.

When I run npm run stencil generate pages/page-home to create a new <page-home> component, the files that get generated are:

$ stencil generate pages/page-home

The following files have been generated:
  - src/components/pages/home/home.tsx
  - src/components/pages/home/home.css
  - src/components/pages/home/home.spec.ts
  - src/components/pages/home/home.e2e.ts
✨  Done in 1.49s.

I was expecting the directory to be src/components/pages/page-home/ and the files to be named the same, e.g. page-home.tsx.

I see from the Styleguide that a prefix is needed to fix the issue, so npm run stencil generate pages/foo-page-home has the desired output of src/components/pages/page-home/page-home.tsx. It seems like the docs need to be updated, but I'm not sure what the proper behavior is. Happy to learn and update them accordingly. Thanks!

erichstark commented 4 years ago

Why is the prefix removed from the component? Are there any reason for that, or it is current issue?

JohnHardy commented 4 years ago

Can @corysmc or @adamdbradley offer some insight? Would you be open to a --keep-prefix?

chris-dura commented 4 years ago

Just adding my thoughts -- I feel that if you're going to provide tooling, utilities, CLIs, etc... the "default" should absolutely dog-food your own rules. So, a generator that violates your own style guide rules out-of-the-box just smells wrong 😅

erichstark commented 4 years ago

Actually with the latest version of stencil this generation of components works oposite. It generates with prefix :D

chris-dura commented 4 years ago

@erichstark -- Yeah, that's what led me to this finding this issue. For some reason, the change was removed, and, imo, the way the generator is working now (violating the style guide by default) is the wrong decision.

stevenbriscoeca commented 4 years ago

wanted to add my two cents that it seems the new generator doesn't seem to follow the style guide because it adds MyButton instead of just Button https://stenciljs.com/docs/style-guide#component-ts-class

Would be good to have the choice to add prefixes or not in the file / folder names

rwaskiewicz commented 1 year ago

Hey all,

I apologize that it took so long for someone on the team to acknowledge this issue. As it stands today, running something like npm run generate -- pages/page-home will generate the following directory structure:

src/components/pages
└── page-home
    ├── page-home.css
    ├── page-home.tsx
    └── test
        ├── page-home.e2e.ts
        └── page-home.spec.tsx

With a component like:

import { Component, Host, h } from '@stencil/core';

@Component({
  tag: 'page-home',
  styleUrl: 'page-home.css',
  shadow: true,
})
export class PageHome {
  // omitted
}

I think there are two things for the Stencil team to consider here:

That is all to say, I'm going to label this to get it ingested into our backlog for the team to take a closer look. Thanks!