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

Feature Request > Generate shared libraries #15

Closed
pascalbe-dev opened this issue Jul 17, 2020 · 8 comments · Fixed by #20
Closed

Feature Request > Generate shared libraries #15

pascalbe-dev opened this issue Jul 17, 2020 · 8 comments · Fixed by #20

Comments

@pascalbe-dev
Copy link

As mentioned in your article series, shared libraries can and should be used to share logic, which does not belong to a specific domain, like technical stuff (authenticaton, ...).

Since the nx ddd plugin adds a lot of constraints to the tslint.json, generating a shared lib via the nrwl schematics would require the developer to add a tag for the type and to add a tag for the domain (--> shared). Otherwise, the shared lib can not be used within the generated libs by this plugin.

I think, it would make sense to add a schematic which creates a shared lib for you with

  • domain:shared type
  • by default within the shared folder --> subfolder can be provided via option (--directory)
  • type can be passed, but has to be any of the layers mentioned in your blog post (util, ui)

In the end, this would enforce a meaningful folder structure and it would free the dev from adding those tags manually. Especially DEVs not used to the nx linting rules might have problems solving the issue, when the tags are missing.

Please let me know, what you think. I'd be happy to support with a PR again.

@pascalbe-dev
Copy link
Author

According to the type: Maybe it makes sense to use a different underlying schematic for type UI and type util. Util libs might also work fine as a nrwl workspace lib and not as an nrwl angular lib. Not sure though.

@manfredsteyer
Copy link
Contributor

I like this idea. I would also go with 2 separate schematics to align with the other ones. In this case we had:

  • ng g [...]:domain
  • ng g [...]:feature
  • ng g [...]:util
  • ng g [...]:ui
  • ng g [...]:api

Can we just use --domain shared to indicate we are referring to the shared kernel?

According to #12, a --directory switch might be useful anyway. I'm wondering what to do if we use --domain x --directory y?

I see two options:

  1. We use domain:x in nx.json and the path /libs/x/y
  2. We use domain:x in nx.json and the path /libs/y

I'd rather go with 1)

What do you think?
Do I miss sth here?

@pascalbe-dev
Copy link
Author

Separate schematics:
Having separate schematics is even better :) Adding an API lib schematic as well sounds also great. The only lib type, which is missing would then be a shell lib for a specific domain.

Shared option:
I think, I would be better to add an extra boolean option to indicate, that a specific util or UI lib is part of the shared kernel. Something like this: ng g [...]:ui --shared or --sharedKernel. This adds one extra flag which is technically not required, but:

  • --domain shared is from the naming point of view not really correct, because the shared stuff is not a domain as far as I understand. These libs just exist for a technical reason (sharing code). They are not related to business logic or the concept of a domain.
  • --domain shared might bring the developer to the idea to use that also in different kinds of libs like feature or domain. But I guess, functionality of feature and/or domain libs should only be shared via an api layer to prevent high coupling between different bounded contexts. So, to summarize that, I think using --domain shared would be a bit misleading. Do I misunderstand something here?

Customize directory:
I agree with you. I think the structure provided with the domain name is really helpful and it should not be overwritten by a directory option. Appending the paths like you mentioned in option 1) sounds great.

Naming of the libs:
What do you think about the naming of the libs. Should they also have a prefix like util-<name> and ui-<name>, e.g. util-authentication?

@manfredsteyer
Copy link
Contributor

Good point. In this case let's go with --shared b/c the sharedKernal is by definition not technical.

If someone is not aware of this, they --domain shared is doing the same, I guess. You are right: shared is not a domain, but technically it is handled like one.

We can also provide a schematic for the shell.

Let's also use the prefixes, you mention (util, ui, api, shell)

@manfredsteyer
Copy link
Contributor

Btw: Nrwl resolved this issue here and demands us to provide an --importPath for buildable libs. This is important for incremental builds b/c the path mappings used by default were not necessarily valid npm package names -- esp. not if there were subfolders involved like in our case here.

E. g. @repo/domain/feature is not valid b/c of the 2nd slash. Because Nrwl doesn't want to guess the valid name, we have to specify it via --importPath.

I's suggest to replace all slashes beginning with the 2nd one with a dash.

E.g. In the case of @repo/domain/feature we would use importPath @repo/domain-feature.

@pascalbe-dev
Copy link
Author

Ah, good to know.

Sounds good.
I'll try to prepare a PR within the next week for the util, ui lib types. I could try to add this importPath adjustment aswell.

@manfredsteyer
Copy link
Contributor

Cool. Looking forward.

Btw: Just played around with it. We need this --importPath online for --publishable; not for --buildable.

@manfredsteyer
Copy link
Contributor

Just released a new version. buildable is now default. Before publishable was default but this demands us now to specify --importPath:

https://github.com/angular-architects/nx-ddd-plugin/releases/tag/1.0.5

This is to avoid breaking changes until we set --importPath to the discussed default value.

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 a pull request may close this issue.

2 participants