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.

Sign up for GitHub

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

Jump to bottom

DO NOT MERGE: Show synthesized triple slash references in diagnostics for top200 analysis #57569

Closed
wants to merge 1 commit into from
Closed

DO NOT MERGE: Show synthesized triple slash references in diagnostics for top200 analysis #57569

wants to merge 1 commit into from

Conversation

andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented Feb 27, 2024 •
edited
Loading

Experiment for follow-up discussions from #57440, #57568

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Feb 27, 2024
@andrewbranch
Copy link
Member Author

@typescript-bot test top200

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 27, 2024 •
edited
Loading

Heya @andrewbranch, I've started to run the diff-based top-repos suite on this PR at 2b69c9f. You can monitor the build here.

Update: The results are in! Part 1, Part 2, Part 3

@typescript-bot
Copy link
Collaborator

@andrewbranch Here are the results of running the top-repos suite comparing main and refs/pull/57569/merge:

Something interesting changed - please have a look.

Details

angular/angular-cli

4 of 22 projects failed to build with the old tsc and were ignored

tests/legacy-cli/tsconfig.json

apollographql/apollo-client

1 of 12 projects failed to build with the old tsc and were ignored

tsconfig.json

BuilderIO/gpt-crawler

tsconfig.json

  • error TS18055: Declaration file contains synthesized type reference directives: '"node"'

cheeriojs/cheerio

1 of 3 projects failed to build with the old tsc and were ignored

tsconfig.json

discordjs/discord.js

40 of 62 projects failed to build with the old tsc and were ignored

packages/brokers/tsconfig.docs.json

packages/brokers/tsconfig.eslint.json

packages/brokers/tsconfig.json

packages/formatters/tsconfig.docs.json

packages/formatters/tsconfig.eslint.json

packages/formatters/tsconfig.json

packages/voice/tsconfig.docs.json

packages/voice/tsconfig.eslint.json

packages/voice/tsconfig.json

Eugeny/tabby

9 of 29 projects failed to build with the old tsc and were ignored

tabby-core/tsconfig.typings.json

tabby-terminal/tsconfig.typings.json

facebook/lexical

1 of 3 projects failed to build with the old tsc and were ignored

tsconfig.build.json

freeCodeCamp/freeCodeCamp

8 of 12 projects failed to build with the old tsc and were ignored

tools/ui-components/tsconfig.json

FuelLabs/fuels-ts

12 of 48 projects failed to build with the old tsc and were ignored

packages/crypto/tsconfig.dts.json

packages/crypto/tsconfig.json

packages/math/tsconfig.dts.json

packages/math/tsconfig.json

packages/fuels/tsconfig.dts.json

homebridge/homebridge

tsconfig.json

immich-app/immich

5 of 8 projects failed to build with the old tsc and were ignored

server/tsconfig.build.json

server/tsconfig.json

ionic-team/ionic-framework

20 of 21 projects failed to build with the old tsc and were ignored

core/tsconfig.json

jhipster/generator-jhipster

1 of 3 projects failed to build with the old tsc and were ignored

tsconfig.types.json

makeplane/plane

6 of 8 projects failed to build with the old tsc and were ignored

packages/editor/core/tsconfig.json

packages/ui/tsconfig.json

@typescript-bot
Copy link
Collaborator

@andrewbranch Here are some more interesting changes from running the top-repos suite

Details

marmelab/react-admin

25 of 26 projects failed to build with the old tsc and were ignored

packages/ra-core/tsconfig.json

nextauthjs/next-auth

16 of 40 projects failed to build with the old tsc and were ignored

packages/adapter-pouchdb/tsconfig.json

nextui-org/nextui

2 of 75 projects failed to build with the old tsc and were ignored

packages/core/system-rsc/tsconfig.json

packages/core/system/tsconfig.json

packages/hooks/use-aria-press/tsconfig.json

packages/components/button/tsconfig.json

packages/components/avatar/tsconfig.json

packages/components/accordion/tsconfig.json

packages/hooks/use-data-scroll-overflow/tsconfig.json

packages/components/scroll-shadow/tsconfig.json

packages/components/listbox/tsconfig.json

packages/components/code/tsconfig.json

packages/components/link/tsconfig.json

packages/components/card/tsconfig.json

packages/components/popover/tsconfig.json

packages/hooks/use-infinite-scroll/tsconfig.json

packages/components/menu/tsconfig.json

packages/components/dropdown/tsconfig.json

packages/components/checkbox/tsconfig.json

packages/components/kbd/tsconfig.json

packages/hooks/use-callback-ref/tsconfig.json

packages/components/modal/tsconfig.json

packages/hooks/use-scroll-position/tsconfig.json

packages/components/navbar/tsconfig.json

packages/components/progress/tsconfig.json

packages/components/radio/tsconfig.json

packages/components/table/tsconfig.json

prisma/prisma

82 of 120 projects failed to build with the old tsc and were ignored

packages/get-platform/tsconfig.build.json

packages/engines/tsconfig.build.json

packages/cli/tsconfig.build.json

packages/client/tsconfig.build.json

pubkey/rxdb

8 of 11 projects failed to build with the old tsc and were ignored

config/tsconfig.types.json

puppeteer/puppeteer

7 of 14 projects failed to build with the old tsc and were ignored

packages/testserver/tsconfig.json

test/installation/tsconfig.json

react-bootstrap/react-bootstrap

2 of 3 projects failed to build with the old tsc and were ignored

tsconfig.json

react-navigation/react-navigation

1 of 14 projects failed to build with the old tsc and were ignored

packages/native/tsconfig.build.json

packages/elements/tsconfig.build.json

packages/react-native-drawer-layout/tsconfig.build.json

packages/drawer/tsconfig.build.json

packages/native-stack/tsconfig.build.json

tsconfig.json

ReactiveX/rxjs

11 of 15 projects failed to build with the old tsc and were ignored

packages/rxjs/src/tsconfig.types.json

recharts/recharts

demo/tsconfig.json

redis/node-redis

packages/client/tsconfig.json

Redocly/redoc

tsconfig.json

tsconfig.lib.json

tailwindlabs/headlessui

1 of 5 projects failed to build with the old tsc and were ignored

packages/@headlessui-react/tsconfig.json

@typescript-bot
Copy link
Collaborator

@andrewbranch Here are some more interesting changes from running the top-repos suite

Details

typeorm/typeorm

tsconfig.json

usablica/intro.js

1 of 3 projects failed to build with the old tsc and were ignored

tsconfig.json

vadimdemedes/ink

1 of 2 projects failed to build with the old tsc and were ignored

tsconfig.json

vercel/hyper

2 of 3 projects failed to build with the old tsc and were ignored

tsconfig.json

vercel/swr

6 of 8 projects failed to build with the old tsc and were ignored

tsconfig.json

  • error TS18056: Declaration file contains synthesized file reference directives: '"../../../env.d.ts"'
  • error TS18055: Declaration file contains synthesized type reference directives: '"react"'

vuetifyjs/vuetify

3 of 7 projects failed to build with the old tsc and were ignored

packages/vuetify/tsconfig.checks.json

packages/vuetify/tsconfig.json

web3/web3.js

55 of 63 projects failed to build with the old tsc and were ignored

packages/web3-types/tsconfig.types.json

youzan/vant

3 of 10 projects failed to build with the old tsc and were ignored

packages/vant-cli/site/tsconfig.json

packages/vant-cli/tsconfig.json

@jakebailey
Copy link
Member

Quite surprised about react showing up that much, given the files I opened all contained an import of react anyway...

@dragomirtitian
Copy link
Contributor

Quite surprised about react showing up that much, given the files I opened all contained an import of react anyway...

Yes that is really surprising. For path references there is code that specifically excludes references to imported files. Guess for type reference directives that logic is missing:

https://github.com/microsoft/TypeScript/blob/main/src/compiler/transformers/declarations.ts#L580-L586

if (exportedModulesFromDeclarationEmit?.includes(file.symbol)) {
    // Already have an import declaration resolving to this file
    return;
}

@andrewbranch
Copy link
Member Author

Looking at these, I don’t feel too worried about the volume of real-world breaks being very high. The only thing I might send a preemptive PR for is adding an explicit reference to "../../env.d.ts" to vercel/swr. That said, knowing you need to do that still feels like a footgun. swr’s tsconfig.json explicitly includes env.d.ts. That could be considered bad practice now, as it sets up a program graph that a library consumer would never have. Libraries should probably set their files array to be just their package.json entrypoints. But even that’s unrealistic, since a consumer might not load all entrypoints. Ideally, you’d want to make sure your library code compiles when setting each package.json entrypoint as the lone tsconfig.json files entry. But nobody is going to do that.

@andrewbranch andrewbranch mentioned this pull request Mar 6, 2024
Closed
@jakebailey jakebailey mentioned this pull request Mar 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@andrewbranch andrewbranch

Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@andrewbranch @typescript-bot @jakebailey @dragomirtitian