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

Use TS references to link monorepo packages #1277

Merged
merged 6 commits into from
Jun 22, 2022
Merged

Conversation

Siegrift
Copy link
Contributor

Closes #1227

This feature enables real time errors and compilation cross-project. This means that when airnode-admin depends on function X in airnode-abi and this function is removed, you will receive instant error in airnode-admin.

We assume that every dependency is used in the monorepo project sources and we require that the tsconfig.json of the sources references all other monorepo dependencies.

When another tsconfig A (e.g. the tests) references the tsconfig B of project sources, the referenced projects in B are already already referenced by A. This means that importing stuff from monorepo packages in tests will still provide the same features as in sources.

@Siegrift Siegrift self-assigned this Jun 21, 2022
@Siegrift Siegrift requested a review from a team June 21, 2022 16:37
@Siegrift Siegrift marked this pull request as ready for review June 21, 2022 17:27
@Siegrift
Copy link
Contributor Author

I've checked how the dist files differ between master and this branch and only differences are the changed build script in package.json and the changed tsbuildinfo (which we are not ignoring, because yarn has a bug with negative file match and we didn't want to migrate to npm. See: yarnpkg/yarn#8332)

Copy link
Contributor

@amarthadan amarthadan left a comment

Choose a reason for hiding this comment

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

Changes look fine to me but I found an issue.

When I, for example, remove the log function from the logger from airnode-utilities package and run a command from airnode-admin package (yarn cli generate-mnemonic) it still works. I would expect it to throw an error since the function for printing was removed from the airnode-utilities package.

@Siegrift Siegrift requested a review from amarthadan June 22, 2022 10:34
@Siegrift
Copy link
Contributor Author

Thanks for the review, I didn't think that ts-node would behave differently. As discussed on slack, ts-node does not support project references (link) so we decided to ignore this issue. It is not a big deal, since the errors are visible in the IDE and are showcased during build (this means also on CI). I've also mentioned this side effect in README.

While investigating this issue I realized there is a setting https://www.typescriptlang.org/tsconfig#tsBuildInfoFile which allows us to move the tsbuildinfo files from the /dist directory which gets published to npm, so I've done that as well.

@Siegrift Siegrift merged commit e34bfb8 into master Jun 22, 2022
@Siegrift Siegrift deleted the link-ts-packages branch June 22, 2022 15:56
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 this pull request may close these issues.

Use TS references for linked monorepo packages
2 participants