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

Don't prefix the app name with nativescript #6

Open
mahmoudajawad opened this issue Apr 3, 2021 · 5 comments
Open

Don't prefix the app name with nativescript #6

mahmoudajawad opened this issue Apr 3, 2021 · 5 comments
Labels
enhancement New feature or request in progress

Comments

@mahmoudajawad
Copy link
Contributor

As README explains:

This will generate:

apps/nativescript-<app-name>

I would suggest that the prefix should be optional, and groupByName option should act on this, not the other way around.

@NathanWalker
Copy link
Contributor

Agreed - will do (Makes sense for apps for this to be purely optional). Curious on your thoughts with libs - as the lib generator also will use a prefix? We've always preferred to have some platform in the organization (around shared code) to be extra clear (especially within large cross platform workspaces).

The problem without (regarding libs) is you could have:

@workspace/ui

However it's not clear at all what UI that's for? Is it for web, nativescript, ionic, vue, what?

@mahmoudajawad
Copy link
Contributor Author

I already use my own prefixes everywhere for every thing--I use ng, ns, py, ...etc., so my actual context is using ns instead of nativescript, which I had to achieve by manually editing the workspace. So, as much as I personally am huge goer for prefixing libs and apps, I'm calling for it to be optional to allow the developer choose the preferred prefix.

@NathanWalker NathanWalker added enhancement New feature or request in progress labels Apr 18, 2021
@mahmoudajawad mahmoudajawad changed the title Don't prefixing the app name with nativescript Don't prefix the app name with nativescript Apr 20, 2021
@AgentEnder
Copy link
Contributor

AgentEnder commented May 7, 2021

What about a option on the generators "affix"? The default would be set to nativescript, but then the team could override the default in workspace.json (see line 56 here https://nx.dev/latest/react/core-concepts/configuration).

Then teams could set it to ns if that is what they prefer, or set it empty string to omit it. Then the plugin was append or prepend it depending on if groupByName was used or omitted.

@mahmoudajawad
Copy link
Contributor Author

@AgentEnder, I applaud this approach. Why, yes, it makes sense as it also streamlines single team/org preferences for the prefix.

marckassay added a commit to marckassay/nx that referenced this issue Feb 24, 2022
marckassay added a commit to marckassay/nx that referenced this issue Feb 25, 2022
@marckassay
Copy link
Contributor

I'm also interested in having this as an option. I made 2 commits (as seen above) that allows this to happen for app and lib creation.

@AgentEnder I attempted to add a new property named 'affix' to the schematic but quickly realized that may be confusing for developers of this repo since there is already a property named 'prefix' that is used for import expressions. The platform property for generators handles the value we are questioning here. I simply moved it to the schematic as a prompt for apps and libs. In doing so, I violated and relaxed some TS types to make this happen.

However, there is a platform property for executors in existing code that is used for mobile platforms (ios, android). Because of the 2 platform properties being used differently and the TS types changed, I don't have any intentions of creating a PR at this moment. I'm hoping @NathanWalker can chime in with his thoughts since he seems to have created this repo, and @mahmoudajawad for creating this post.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress
Projects
None yet
Development

No branches or pull requests

4 participants