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

Add type discovery to src symlink directories #4089

Merged
merged 1 commit into from
Aug 15, 2023
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Aug 15, 2023

We currently don't show TypeScript tsc compiler issues for our previous source code symbolic links:

  • src/govukpackages/govuk-frontend/src/govuk
  • src/govuk-prototype-kitpackages/govuk-frontend/src/govuk-prototype-kit

This PR adds a new tsconfig.json file to fix that

But to avoid running our checks twice we can exclude it from references by default:

"references": [

This ensures TypeScript `tsc` compiler issues appear in code editors and IDEs when our previous source code symbolic links are followed:

* `src/govuk` → `packages/govuk-frontend/src/govuk`
* `src/govuk-prototype-kit` → `packages/govuk-frontend/src/govuk-prototype-kit`
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4089 August 15, 2023 10:29 Inactive
@colinrotherham colinrotherham changed the title [SPIKE] Add type discovery to src directory [SPIKE] Add type discovery to src symlink directories Aug 15, 2023
@colinrotherham
Copy link
Contributor Author

Alternatively we may want to delete the symbolic links entirely but see #3491 (review)

  • we should add a way, if only for a while, to direct people to the new places, as they're used to looking at the sources in src and the app in app. We mentioned README.md files in the old locations, which sound an easy way to sort this out, or symlinks. Does any have particular drawbacks (maybe in terms of rebasing)? I think these pointers should be part of this PR so all the info is available once merged.

@colinrotherham colinrotherham changed the title [SPIKE] Add type discovery to src symlink directories Add type discovery to src symlink directories Aug 15, 2023
Copy link
Member

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Thanks for coming up with a fix 🙌🏻 Let's go with this for now. If we see other issues caused by the symlinks we can revisit.

@colinrotherham colinrotherham merged commit ad03969 into main Aug 15, 2023
45 checks passed
@colinrotherham colinrotherham deleted the tsconfig-src branch August 15, 2023 11:17
colinrotherham added a commit that referenced this pull request Aug 22, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` the default “build only” type checks didn’t include Jest globals etc

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved

We previously enabled type discovery in:
#4089
colinrotherham added a commit that referenced this pull request Aug 22, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` the default “build only” type checks didn’t include Jest globals etc

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved

We previously enabled type discovery in:
#4089
colinrotherham added a commit that referenced this pull request Aug 22, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` globals for Jest etc weren't supported:

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved. We previously enabled type discovery in:

* #4089
colinrotherham added a commit that referenced this pull request Aug 23, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` globals for Jest etc weren't supported:

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved. We previously enabled type discovery in:

* #4089
colinrotherham added a commit that referenced this pull request Aug 23, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` globals for Jest etc weren't supported:

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved. We previously enabled type discovery in:

* #4089
colinrotherham added a commit that referenced this pull request Aug 23, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` globals for Jest etc weren't supported:

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved. We previously enabled type discovery in:

* #4089
colinrotherham added a commit that referenced this pull request Aug 23, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` globals for Jest etc weren't supported:

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved. We previously enabled type discovery in:

* #4089
colinrotherham added a commit that referenced this pull request Aug 24, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` globals for Jest etc weren't supported:

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved. We previously enabled type discovery in:

* #4089
colinrotherham added a commit that referenced this pull request Sep 27, 2023
This commit brings changes from 8b20a4d to `./src` symlink directories

When accidentally opening a Jest test or Gulp task file inside `./src` globals for Jest etc weren't supported:

```
describe()
beforeAll()
beforeEach()
it()
```

This is now resolved. We previously enabled type discovery in:

* #4089
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants