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

TypeScript module resolution and lint updates #2309

Merged
merged 2 commits into from
Jan 31, 2020

Conversation

Cammisuli
Copy link
Member

@Cammisuli Cammisuli commented Jan 13, 2020

Please make sure you have read the submission guidelines before posting an PR

Please make sure that your commit message follows our format.

Example: fix(nx): must begin with lowercase

Current Behavior (This is the behavior we have today, before the PR is merged)

Imports are found via string comparison.

Expected Behavior (This is the new behavior we can expect after the PR is merged)

Imports are found via TypeScript then fallback to the previous implementation. Project nodes are also sorted properly sorted by:

  • Longest root path first
  • npm nodes last

This also includes updates to linting, and will find more dependency edges because of TypeScript.

Previously, we could import relatively to a file in an app like so (and telling tslint to ignore imports into libs): ../../../apps/myapp/environment/environment.ts.

Doing this will now add that app a dependency to the project using it. This could potentially cause linting errors with circular dependencies.

Issue

@Cammisuli
Copy link
Member Author

Cammisuli commented Jan 13, 2020

Tested the changes with ocean and nx-examples. We will need to update ocean to remove circular dependencies. Nx-examples ran fine.

If there's another nx repo that's fairly mature, I can test it there as well.

@Cammisuli Cammisuli force-pushed the target-project-locator-fixes branch 3 times, most recently from 6176261 to 4cdcfb0 Compare January 16, 2020 16:31
@Cammisuli Cammisuli changed the title Target project locator fixes TypeScript module resolution and lint updates Jan 16, 2020
@Cammisuli Cammisuli force-pushed the target-project-locator-fixes branch 3 times, most recently from 5ff8801 to ab14b43 Compare January 20, 2020 22:37
@FrozenPandaz FrozenPandaz added the PR status: breaking-changes PR contains breaking changes label Jan 23, 2020
This will sort nodes by length of the root (high to low) then nodes that have no root.

It also uses TypeScript to first try and resolve a module. If it is not found via TypeScript, it will
fall back to using a string match.
@FrozenPandaz FrozenPandaz merged commit 1c40789 into nrwl:master Jan 31, 2020
@Cammisuli Cammisuli deleted the target-project-locator-fixes branch January 29, 2021 15:49
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR status: breaking-changes PR contains breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants