modular source type

[cristiano-belloni]

modular.type: source and --dangerouslyIgnoreCircularDependencies

This PR introduces the source type (i.e., a package that can be imported but never gets built or started)

Why we are considering source packages

The source type is useful for all those packages that just get imported from app, esm-view or package workspaces and never get built (ie common utils or views only used internally). When the package type is used for these kind of workspaces, a confusing situation arises, in which these packages should never be built standalone (they can even fail, since they're often intended to be built as part of apps or esm-views and might need build-time plugins) but still get built by selective build operators (for example, --changed will try to incrementally build all changed Modular workspaces). There are two ways of avoiding this: disallowing direct imports from source via eslint rules or introducing the source type. The first solution is not really practical, since many pre-existing projects are already set-up to import other packages from source (and you probably shouldn't be forced to build all your common utils everytime), so the source type could be an useful solution to the problem.

When to convert to source packages

If your workspace:

1. has type package*
2. is imported by more than one app, esm-view or package type in your monorepo**
3. is always imported from its source, not from its dist directory
4. doesn't ever need to be built standalone (e.g. it's never published to a package registry or only its code is)

your workspace is a good candidate to be converted to the source type

* You can also convert a view to source, but you will lose the possibility of modular starting it.
** If it's imported by only one workspace and it's not published as per 4), you might consider incorporating it into the importing package?

source packages and circular dependencies

Normally modular build throws when there are circular dependencies in the workspace dependencies graph, since it can't calculate the correct build order. Since source types don't build, however, it's possible for Modular to work with circular dependencies, if the circularity depends on source packages (i.e., if, removing source packages from the graph, the cycle[s] disappear). We require the flag --dangerouslyIgnoreCircularDependencies to be set in that case.

Why we are considering allowing circular dependencies

Circular dependencies should not be used; they mess up the require order, they can confuse developer tools and they are always fixable by creating additional dependencies which contain the common parts.
When Modular analyses dependencies to build them in order, the algorithm can't decide what to build first when it encounters a circular dependency (e.g.: if A depends on B but B depends on A, how do you know which dist files are needed first?), and it throws. Even if A imports B/a-module and B depends on A/some-module (so the circularity can technically "work", because the code that imports a-module isn't the same code that is imported by B, and vice-versa), it's impossible to determine that that is the case by just statically analysing the code. Additionally, the resulting code is a nightmare to maintain (if you refactor files in A so that import and import-ed are in the same file, the build chain comes crashing down). If possible, projects should not use circular dependencies, and Modular might to want warn in the future when it detects them.

This PR shows that the Modular order algorithm can allow circular dependencies if we introduce a new modular.type called source, which signals that the package doesn't need to (and cannot) be built. If we have a circular dependency involving a source package (i.e. if B is source in the previous example), we can still calculate a valid build order by removing sources from the order graph, since they don't get built and nothing can depend on their build result anyway. This is not to encourage circular deps, but to allow huge projects that unfortunately already have them to be onboarded without extensive refactoring.

Examples

Cycle disappearing when source types are removed from the dependency graph (cycle between package and source)

(assuming that package b is depending on source c and source c is
depending on package b and package d):

Without --dangerouslyIgnoreCircularDependencies the build fails

> modular-dev build b --descendants
[modular] Building packages at: b
[modular] Cycle detected, b -> c -> b

With --dangerouslyIgnoreCircularDependencies the build warns but succeeds

> modular-dev build b --descendants --dangerouslyIgnoreCircularDependencies
[modular] Building packages at: b
[modular] You chose to dangerously ignore cycles in the dependency graph. Builds will still fail if a cycle is found involving two or more buildable packages. Please note that the use of this flag is not recommended. It's always possible to break a cyclic dependency by creating an additional dependency that contains the common code.
[modular] $ b: generating .d.ts files
[modular] $ b: building b...
[modular] $ b: built b in /Users/N761472/dev/rig/ghost-building/dist/b
[modular] $ d: generating .d.ts files
[modular] $ d: building d...
[modular] $ d: built d in /Users/N761472/dev/rig/ghost-building/dist/d

Cycle not disappearing when source types are removed from the dependency graph (cycle between packages)

(assuming that package b is depending on package c and package c is
depending on package b and package d):

Even with --dangerouslyIgnoreCircularDependencies the build fails:

> modular-dev build b --descendants --dangerouslyIgnoreCircularDependencies
[modular] Building packages at: b
[modular] You chose to dangerously ignore cycles in the dependency graph. Builds will still fail if a cycle is found involving two or more buildable packages. Please note that the use of this flag is not recommended. It's always possible to break a cyclic dependency by creating an additional dependency that contains the common code.
[modular] Cycle detected, b -> c -> b

TODO

• implement source type
• implement removal from graph
• implement --dangerouslyIgnoreCircularDependencies
• document circular dependencies
• document source type
• test source type
• test --dangerouslyIgnoreCircularDependencies

[changeset-bot]

🦋 Changeset detected

Latest commit: a6ffa4d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
modular-scripts	Minor

Not sure what this means? Click here to learn what changesets are.

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

[coveralls]

Coverage decreased (-0.6%) to 32.65% when pulling a6ffa4d on feature-source-type into 4a541a9 on main.

[sgb-io]

Looks good so far

[sgb-io]

Ideal content to go in the markdown docs as a table

[AlbertoBrusa]

Looks good to me

[AlbertoBrusa]

Approved, but wondering if we should PR into feature/v4 and fixing the conflicts (if any) now- having said that I don't mind merging to main and then pulling main into v4 again, not a huge effort

[sgb-io]

LGTM, just 1 comment about some possible debug artefacts

[sgb-io]

Are these console.log statements intentional?