modular source type (#2228)

* Allow circular dependencies for source packages

* Remove non-buildable workspaces from build anyway

* Better package types utilities

* Typos + fix test

* Better start check and error message

* Remove default export

* Simplify Modular type checks

* Split selectWorkspaces

* Introduce --dangerouslyIgnoreCircularDependencies

* Add circular dependencies docs

* Add fixtures and tests

* Add source type to add command + docs

* update snapshots

* Create short-laws-stare.md

* Fix typo

.changeset/short-laws-stare.md

@@ -0,0 +1,5 @@
+---
+"modular-scripts": minor
+---
+
+modular `source` type + `--dangerouslyIgnoreCircularDependencies` build option

__fixtures__/source-type/.editorconfig

@@ -0,0 +1,18 @@
+# https://editorconfig.org
+root = true
+
+[*]
+charset = utf-8
+end_of_line = lf
+indent_size = 2
+indent_style = space
+insert_final_newline = true
+max_line_length = 80
+trim_trailing_whitespace = true
+
+[*.{md,mdx}]
+max_line_length = 0
+trim_trailing_whitespace = false
+
+[COMMIT_EDITMSG]
+max_line_length = 0

__fixtures__/source-type/.eslintignore

@@ -0,0 +1,23 @@
+# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.
+
+# dependencies
+node_modules
+/.pnp
+.pnp.js
+
+# testing
+/coverage
+
+# misc
+.DS_Store
+.env.local
+.env.development.local
+.env.test.local
+.env.production.local
+
+npm-debug.log*
+yarn-debug.log*
+yarn-error.log*
+
+packages/**/public
+/dist

__fixtures__/source-type/.gitignore

@@ -0,0 +1,24 @@
+# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.
+
+# dependencies
+node_modules
+/.pnp
+.pnp.js
+
+# testing
+/coverage
+
+# misc
+.DS_Store
+.env.local
+.env.development.local
+.env.test.local
+.env.production.local
+
+npm-debug.log*
+yarn-debug.log*
+yarn-error.log*
+
+/dist
+
+.vscode

__fixtures__/source-type/.prettierignore

@@ -0,0 +1,2 @@
+/dist
+/packages/**/public

__fixtures__/source-type/.yarnrc

@@ -0,0 +1 @@
+disable-self-update-check true

__fixtures__/source-type/README.md

@@ -0,0 +1 @@
+This is the `README.md` for the whole monorepo.

__fixtures__/source-type/modular/setupEnvironment.ts

@@ -0,0 +1,2 @@
+// Allows for adding setup configuration to Jest
+export {};

__fixtures__/source-type/modular/setupTests.ts

@@ -0,0 +1,5 @@
+// jest-dom adds custom jest matchers for asserting on DOM nodes.
+// allows you to do things like:
+// expect(element).toHaveTextContent(/react/i)
+// learn more: https://github.com/testing-library/jest-dom
+import '@testing-library/jest-dom/extend-expect';

__fixtures__/source-type/package.json

@@ -0,0 +1,60 @@
+{
+  "name": "ghost-building",
+  "version": "1.0.0",
+  "main": "index.js",
+  "author": "Cristiano Belloni <cristiano.belloni@jpmorgan.com>",
+  "license": "MIT",
+  "private": true,
+  "workspaces": [
+    "packages/**"
+  ],
+  "modular": {
+    "type": "root"
+  },
+  "scripts": {
+    "start": "modular start",
+    "build": "modular build",
+    "test": "modular test",
+    "lint": "eslint . --ext .js,.ts,.tsx",
+    "prettier": "prettier --write ."
+  },
+  "eslintConfig": {
+    "extends": "modular-app"
+  },
+  "browserslist": {
+    "production": [
+      ">0.2%",
+      "not dead",
+      "not op_mini all"
+    ],
+    "development": [
+      "last 1 chrome version",
+      "last 1 firefox version",
+      "last 1 safari version"
+    ]
+  },
+  "prettier": {
+    "singleQuote": true,
+    "trailingComma": "all",
+    "printWidth": 80,
+    "proseWrap": "always"
+  },
+  "dependencies": {
+    "@testing-library/dom": "^8.19.0",
+    "@testing-library/jest-dom": "^5.16.5",
+    "@testing-library/react": "^13.4.0",
+    "@testing-library/user-event": "^7.2.1",
+    "@types/jest": "^29.2.3",
+    "@types/node": "^18.11.9",
+    "@types/react": "^18.0.25",
+    "@types/react-dom": "^18.0.9",
+    "eslint-config-modular-app": "^3.0.2",
+    "modular-scripts": "^3.5.0",
+    "modular-template-app": "^1.1.0",
+    "modular-template-package": "^1.1.0",
+    "prettier": "^2.7.1",
+    "react": "^18.2.0",
+    "react-dom": "^18.2.0",
+    "typescript": ">=4.2.1 <4.5.0"
+  }
+}

__fixtures__/source-type/packages/a/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "a",
+  "private": false,
+  "modular": {
+    "type": "package"
+  },
+  "main": "./src/index.ts",
+  "version": "1.0.0",
+  "dependencies": {
+    "e": "1.0.0"
+  }
+}

__fixtures__/source-type/packages/a/src/index.ts

@@ -0,0 +1,3 @@
+export default function add(a: number, b: number): number {
+  return a + b;
+}

__fixtures__/source-type/packages/b/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "b",
+  "private": false,
+  "modular": {
+    "type": "package"
+  },
+  "main": "./src/index.ts",
+  "version": "1.0.0",
+  "dependencies": {
+    "c": "1.0.0"
+  }
+}

__fixtures__/source-type/packages/b/src/index.ts

@@ -0,0 +1,3 @@
+export default function add(a: number, b: number): number {
+  return a + b;
+}

__fixtures__/source-type/packages/c/package.json

@@ -0,0 +1,13 @@
+{
+  "name": "c",
+  "private": false,
+  "modular": {
+    "type": "source"
+  },
+  "main": "./src/index.ts",
+  "version": "1.0.0",
+  "dependencies": {
+    "b": "1.0.0",
+    "d": "1.0.0"
+  }
+}

__fixtures__/source-type/packages/c/src/index.ts

@@ -0,0 +1,3 @@
+export default function add(a: number, b: number): number {
+  return a + b;
+}

__fixtures__/source-type/packages/d/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "d",
+  "private": false,
+  "modular": {
+    "type": "source"
+  },
+  "main": "./src/index.ts",
+  "version": "1.0.0"
+}

__fixtures__/source-type/packages/d/src/index.ts

@@ -0,0 +1,3 @@
+export default function add(a: number, b: number): number {
+  return a + b;
+}

__fixtures__/source-type/packages/e/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "e",
+  "private": false,
+  "modular": {
+    "type": "package"
+  },
+  "main": "./src/index.ts",
+  "version": "1.0.0",
+  "dependencies": {
+    "a": "1.0.0"
+  }
+}

__fixtures__/source-type/packages/e/src/index.ts

@@ -0,0 +1,3 @@
+export default function add(a: number, b: number): number {
+  return a + b;
+}

__fixtures__/source-type/tsconfig.json

@@ -0,0 +1,4 @@
+{
+  "extends": "modular-scripts/tsconfig.json",
+  "include": ["modular", "packages/**/src"]
+}

docs/commands/add.md

@@ -28,12 +28,18 @@ Packages can currently be one of the following types:
   architecture or started as a normal standalone application. See also
   [the view building reference](../esm-views/index.md)
 
-- A `view`, which is a package that exports a React component by default. Read
+- A `view`, which is a `package` that exports a React component by default. Read
   more about Views in [this explainer](../concepts/views.md).
 
-- A generic JavaScript `package`. You can use this to create any other kind of
-  utility, tool, or whatever your needs require you to do. As an example, you
-  could build a node.js server inside one of these.
+- A generic JavaScript `package`. You can use this to create a library with an
+  entry point that gets transpiled to Common JS and ES Module format when built.
+  Packages can be [built](../commands/build.md) but not
+  [start](../commands/start.md)ed by Modular.
+
+- A `source`, which is a shared package that is imported by other packages from
+  source (i.e. directly importing its source), and it's never built standalone
+  or published. This kind of package is never [built](../commands/build.md) or
+  [start](../commands/start.md)ed by Modular.
 
 ## Options:
 

docs/concepts/circular-dependencies.md

@@ -0,0 +1,78 @@
+---
+title: Circular Dependencies
+parent: Concepts
+---
+
+# Circular dependencies and Modular
+
+`modular build` always tries to calculate the build order based on the workspace
+dependency graph. If it encounters a cyclic dependency during this calculation,
+it will bail out, since the build order can't be determined (if A depends on the
+build result of B and B depends on the build result of A, which one should be
+get built first?)
+
+Circular dependencies should never be introduced in your Modular monorepository;
+apart from interfering with the calculation of build order, they can
+[lead to unexpected results in require order](https://nodejs.org/api/modules.html#cycles),
+they can confuse developer tools and they are always fixable
+[by creating additional dependencies which contain the common parts](https://nx.dev/recipes/other/resolve-circular-dependencies).
+Even you manage to avoid all of those issues, circular dependencies can make
+refactoring fragile: if a part of A depends on a part of B and an unrelated part
+of B depends on an unrelated part of A, refactoring code in A can make the
+require order in B fail and vice versa.
+
+## `--dangerouslyIgnoreCircularDependencies` escape hatch
+
+If your circular dependencies involve packages that never get built (namely,
+`modular.type: source` packages), Modular can still calculate the correct build
+order by removing them from the dependency graph. By default, `modular build`
+will still refuse to build any build graph that contains a circular dependency,
+but this behaviour can be overridden by specifying the
+`--dangerouslyIgnoreCircularDependencies` flag. Please note that `modular build`
+will still fail if the cycle involves two or more buildable (i.e. non-`source`)
+packages. This doesn't solve all the other issues linked to
+`--dangerouslyIgnoreCircularDependencies`, so please don't use this flag in
+production and always split you dependencies to avoid cycles.
+
+## 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
+
+```bash
+> modular-dev build b --descendants
+[modular] Building packages at: b
+[modular] Cycle detected, b -> c -> b
+```
+
+#### With `--dangerouslyIgnoreCircularDependencies` the build warns but succeeds
+
+```bash
+> 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 `package`s)
+
+(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:
+
+```bash
+> 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
+```

packages/modular-scripts/src/__tests__/selectiveBuild.test.ts

@@ -60,7 +60,7 @@ describe('--changed builds all the changed packages in order', () => {
       '--changed',
     ]);
     expect(result.stderr).toBeFalsy();
-    expect(result.stdout).toContain('No changed workspaces found');
+    expect(result.stdout).toContain('No workspaces to build');
   });
 
   it('builds multiple packages', () => {