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

feat[Angular]: Allow passing explicit selectors #1443

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

seofernando25
Copy link

@seofernando25 seofernando25 commented May 11, 2024

Description

Adds an explicit, selector option, allowing to override the default,

selector: `'${kebabCase(json.name || 'my-component')}, ${json.name}'`

This doesn't quite work if you have more than one component per project since all of them would have the same name

selector: options.selector
        ? `'${options.selector}'`
       : `'${kebabCase(json.name || 'my-component')}, ${json.name}'`,

Another option could try is to keep as is and leave the warning there.

Closes #1442

Also see: angular's style guide
Relevant PR that added it in the first place

EDIT: Ok I think I can see why it was added there in the first place... might be wise to allow overriding the selector

Copy link

changeset-bot bot commented May 11, 2024

⚠️ No Changeset found

Latest commit: fac4711

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@seofernando25 seofernando25 marked this pull request as ready for review May 11, 2024 10:50
@seofernando25
Copy link
Author

seofernando25 commented May 11, 2024

~~I'm having some issues running the snapupdates. ~~
Is it suppose that take this long 10+ minutes?

So far I've changed the expected results manually.

SOLUTION: Manually ran vitest file.test.ts -u for each failing test

@seofernando25 seofernando25 marked this pull request as draft May 12, 2024 23:25
@seofernando25 seofernando25 changed the title fix: Remove name from Angular selector name feat: Allow passing explicit selectors May 12, 2024
@seofernando25 seofernando25 changed the title feat: Allow passing explicit selectors feat[Angular]: Allow passing explicit selectors May 12, 2024
selector: `'${kebabCase(json.name || 'my-component')}, ${json.name}'`,
selector: options.selector
? `'${options.selector}'`
: `'${kebabCase(json.name || 'my-component')}, ${json.name}'`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to remove the ${json.name} as default.

If a users wants to overwrite the selector with non kebabCase they may use options.selector.

@seofernando25 seofernando25 marked this pull request as ready for review May 22, 2024 19:14
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 this pull request may close these issues.

Angular selector does not follow angular selector names
2 participants