#9430: tsconfig references refactor

[fungairino]

What does this PR do?

• Closes Slice 2: Add nx initial config and commands #9430
• Sets up incremental TS builds for our monorepo to speed up typescript compilation and development. See: https://www.typescriptlang.org/docs/handbook/project-references.html

Discussion

• We could go with a "buildless" approach for our TS configuration but this over the long run will make our type checking a lot slower

Demo

• Verified that tsc --build --watch recompiles only the utils directory when changes are made to it, and leaves the whole extension project untouched

Future Work

• Add @nx/js plugin to handle ts tasks and auto updating tsconfig references
• Make our eslint config situation better

Checklist

• FIx lint issue
• Fix knip issue

For more information on our expectations for the PR process, see the
code review principles doc

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.79%. Comparing base (8318d74) to head (f6bfacf).
Report is 507 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9522      +/-   ##
==========================================
+ Coverage   74.24%   75.79%   +1.54%     
==========================================
  Files        1332     1422      +90     
  Lines       40817    42917    +2100     
  Branches     7634     7902     +268     
==========================================
+ Hits        30306    32529    +2223     
+ Misses      10511    10388     -123     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Playwright test results

 152 passed
 2 flaky
 2 skipped

Details

 Open report ↗︎
 156 tests across 51 suites
 11 minutes, 18 seconds
 f6bfacf
 For more information on how to debug and view this report, see our readme

Flaky tests

msedge › tests/runtime/localIntegrations.spec.ts › Local Integrations Page › #8067: blank numeric text integration configuration field validated on save
msedge › tests/workshop/createMod.spec.ts › can create a new mod from a yaml definition and update it

Skipped tests

chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
msedge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options

[twschiller]

What's the import convention from inside the browser-extension workspace? Or do we have to wait for @nx/js to set that up?

[fungairino]

This is imported in the browser-extension like so:
import { getInvalidPath } from "@pixiebrix/utils/src/debugUtils";

I removed this following the suggested tsconfigs from nx, but I think we could add back the following in the libraries/utils tsconfig if we wanted:

  "compilerOptions": {
    "paths": {
      "@/*": ["src/*"]

I don't feel too strongly either way about this.

[twschiller]

I'm not a big fan of the src directory appearing in the import, but it wouldn't be deal breaker. @pixiebrix/utils/debugUtils would seem the cleanest to me (like you're importing a normal package)

I'm generally not a fan of relative imports (for reasons, e.g., listed on the lint rule https://www.npmjs.com/package/eslint-plugin-no-relative-import-paths), but with the same directory a potential exception

Where in the slicing are we introducing an actual cross-workspace import to the build?

Update: I see that the example import is here:

pixiebrix-extension/applications/browser-extension/src/pageEditor/store/editor/editorSlice.ts

Line 771 in 0c76026

invalidPath: getInvalidPath(

See comment on top-level npm run watch: #9522 (comment)

[fungairino]

I'm not a big fan of the src directory appearing in the import, but it wouldn't be deal breaker. @pixiebrix/utils/debugUtils would seem the cleanest to me (like you're importing a normal package)

This should be simple enough to update with an exported index file as a follow-up

[twschiller]

@fungairino I'm getting an error on top-level npm run watch. Am I supposed to be able to run that? (Would that require defining a watch target in the library to also have the tsc watch the files there? Is this related to your comment: "We could go with a "buildless" approach for our TS configuration...")

src/pageEditor/store/editor/editorSlice.ts:37:32 - error TS6305: Output file '/Users/tschiller/projects/pixiebrix-extension/libraries/utils/dist/debugUtils.d.ts' has not been built from source file '/Users/tschiller/projects/pixiebrix-extension/libraries/utils/src/debugUtils.ts'.

37 import { getInvalidPath } from "@pixiebrix/utils/src/debugUtils";

SO chatter here: https://stackoverflow.com/questions/55115705/typescript-project-with-references

IntelliJ can resolve the usage. But typescript not happy about the type:

[fungairino]

I'm getting an error on top-level npm run watch

See discussion in slack https://pixiebrix.slack.com/archives/C080KB9R76G/p1731595386835629

Devs will need to have a running tsc watch process to see updates from underlying libraries in apps if they are making changes across projects concurrently. I'll make sure to communicate this out.

I pushed up a change to update the root level watch script so it should now correctly run tsc progressively across the projects in the monorepo.

[github-actions]

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

[grahamlangford]

Should we really be making every file in the src folder an entry? What if a utility function exports a helper function for testing but shouldn't be used outside the library?

Suggested change

	"libraries/*": {
	entry: "src/*.ts!",
	project: "**/*.ts",
	},
	"libraries/*": {
	// Files with a `!` suffix are included in production mode and default run
	entry: "src/index.ts!",
	project: ["src/**/*.ts!", "!src/**/*.d.ts!"],
	},

This way we force engineers to define in the index.ts exactly what functions are available outside the library

Second, NX recommends we call the library either util or util-*: https://nx.dev/concepts/decisions/project-dependency-rules#utility-libraries

[grahamlangford]

I tested the original code by creating a testFn.ts that exported a simple function and a testFn.test.ts that imported it. It's used nowhere else. The current config doesn't catch it when running production mode. The suggested config does.

[grahamlangford]

Wait, I checked whether the @internal disable would work, looks like it doesn't

Works with a more realistic test scenario (function exported from debugUtils but not index.ts instead)

[fungairino]

I totally agree with this suggestion, I will make this change in a follow-up since this aligns with the same feedback that Todd left further up on avoiding importing from the src directory.