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(js): provide a new way of generating project name and root directory for libraries #18420

Merged

Conversation

leosvelperez
Copy link
Member

@leosvelperez leosvelperez commented Aug 1, 2023

Current Behavior

The @nx/js:library generator (and all other Nx first-party plugins' project generators) generates the project name, directory, and import path in the following way (rough guidelines):

  • project name: [directory-]name (directory path separators replaced with -)
    • additionally, the name can only start with a letter
  • project root: [layoutDir/][directory/]name
  • import path:
    • if --importPath is provided is used
    • if not provided: [@npmScope/]projectRoot (projectRoot without the layoutDir)

Those rules impose certain limitations:

  • If there's a layout dir (workspaceLayouts in nx.json) configured, it will always be used. There's no way to opt out of it for certain projects
  • The generated directory is always formed by concatenating the provided directory and the project name
  • Can't generate the project name matching the import path (package name, e.g. @my-org/my-pkg)
  • The calculated import path (when not provided), can be an invalid npm package name (most of the time is not an issue, to generate a publishable library an --importPath is required to be provided)

Expected Behavior

There should be a way to generate exactly what the user provides to the generator. To avoid breaking changes, this will be behind a flag.

nx g @nx/js:library lib-name --directory shared --project-name-and-root-format=derived
# project name: shared-lib-name
# project root: libs/shared/lib-name or shared/lib-name (depending on the workspace layout dirs)
# import path: @my-org/shared/lib-name (assuming 'my-org' is the npmScope)

nx g @nx/js:library lib-name --directory shared --project-name-and-root-format=as-provided
# project name: lib-name
# project root: shared (if you still want shared/lib-name, you can explicitly provide it)
# import path: @my-org/lib-name
nx g @nx/js:library @my-org/pkg-name --project-name-and-root-format=derived
# throws an error, that's not supported in the current way

nx g @nx/js:library @my-org/pkg-name --project-name-and-root-format=derived
# project name: @my-org/pkg-name
# project root: @my-org/pkg-name (if you want a different directory, you can explicitly provide it)
# import path: @my-org/pkg-name

The new way is simpler to reason about. There are almost no rules or hidden magic you need to be aware of to understand why things were generated in a certain way. What you provide is what you get.

The only rules that apply are:

  • if --directory is not provided, the directory will be the project name
  • if --importPath is not provided, it's generated as follows: [@npmScope/]name

Notes about --project-name-and-root-format and compatibility

This is intended to be a non-breaking change. At the same time, we want to make it as visible as possible and prompt for which format to use. The following are some implementation details and context to achieve both things:

  • The --project-name-and-root-format generator option doesn't have an explicit default value in the schema. The default value is dynamically set or the generator will prompt for the value.
  • The generator uses a default value when:
    • Running the generation in non-interactive mode: defaults to derived (a.k.a. the current behavior)
    • Invoked from third-party generators: defaults to derived (a.k.a. the current behavior)
  • The generator prompts for the format to use when:
    • Running in interactive mode: prompts to choose how to generate
    • Invoked from first-party generators: prompts to choose how to generate (this will be done in follow-up PRs)

The above are the rules followed when the option is not provided to the generator. When the option is provided, it is respected as with any other option.

Related Issue(s)

Fixes #

@leosvelperez leosvelperez self-assigned this Aug 1, 2023
@vercel
Copy link

vercel bot commented Aug 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 10, 2023 11:28am

@leosvelperez leosvelperez force-pushed the misc/consistent-directory-generation branch from 22fc6a7 to 4c347f3 Compare August 3, 2023 09:12
@leosvelperez leosvelperez force-pushed the misc/consistent-directory-generation branch from 7d6d59f to ab4fb04 Compare August 3, 2023 12:59
@leosvelperez leosvelperez changed the title feat(misc): provide a new way of generating project names and directories feat(js): provide a new way of generating project names and directories for libraries Aug 3, 2023
@leosvelperez leosvelperez marked this pull request as ready for review August 3, 2023 14:07
@leosvelperez leosvelperez requested review from a team as code owners August 3, 2023 14:07
Copy link
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

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

Looks good 🎉 - I don't see anything major that needs to be changed, but I think we should really consider checking the generator name. Let me know your thoughts 😄

e2e/nx-run/src/affected-graph.test.ts Outdated Show resolved Hide resolved
packages/js/src/generators/library/library.ts Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I didn't realize this generator was still kicking around 🤔

@FrozenPandaz do you think we can remove it soon?

Copy link
Contributor

Choose a reason for hiding this comment

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

@meeroslav marked it deprecated for the next version.
#18464

Copy link
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

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

Looks good 🎉 - I don't see anything major that needs to be changed, but I think we should really consider checking the generator name. Let me know your thoughts 😄

nameDirectoryFormat: options.nameDirectoryFormat,
rootProject: options.rootProject,
});
options.rootProject = projectDirectory === '.';
Copy link
Collaborator

Choose a reason for hiding this comment

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

options.rootProject would only be used when nameDirectoryFormat is derived right? So when we remove the derived option, we can also remove rootProject? 🤞

Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly. If rootProject is provided, is considered for both formats. It's only relevant if the directory was not provided though.

If we remove it when derived is removed, we lose the ability to create what we call a root project. We decided to use the project name as the directory if the directory is not provided for the new way. So, if rootProject doesn't exist, you can't create a project in the root because you'll always get a folder with the project name.

packages/js/src/generators/library/library.ts Show resolved Hide resolved
packages/js/src/generators/library/schema.json Outdated Show resolved Hide resolved
packages/js/src/generators/library/schema.json Outdated Show resolved Hide resolved
@leosvelperez leosvelperez changed the title feat(js): provide a new way of generating project names and directories for libraries feat(js): provide a new way of generating project name and root directory for libraries Aug 10, 2023
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants