chore: use import type syntax
#17864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for taking the time to open a PR!
|
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
lmiller1990
changed the title
import type syntax across entire repo
|
@lmiller1990 Seems to be quite a bit of failures in this branch. Could you look into those? |
|
@jennifer-shehane actually just reporter is failing, I think the rest is flake -- gimme an hour. Thanks for the quick response. |
|
@jennifer-shehane 1 hour has passed, as promised we are all green ✅ |
tgriesser
previously approved these changes
Aug 24, 2021
tgriesser
approved these changes
Aug 24, 2021
flotwig
approved these changes
Aug 24, 2021
lmiller1990
changed the title
import type syntax across entire repoimport type syntax
tgriesser
added a commit
that referenced
this pull request
Sep 1, 2021
…p-gui-gql-stitching * unified-desktop-gui: fix: rename testApolloClient -> testUrqlClient, add apollo.config.js (#17948) feat(launchpad): detect if runner is set up and change UI accordingly (#17934) rebuild yarn lock chore: remove skipLibCheck, re-add vue-tsc, fix types (#17924) fix: ensure @headlessui/vue is bundled chore(graphql): make tsconfig more strict (#17909) run vite during watch fix: source maps while developing electron rebuild schema w/o JS files make constant more generic remove hanging tests allow larger artifacts comment out flaky test fix typing errors make graphql tsconfig more minimal chore: use `import type` syntax (#17864) remove browser-specific `beforeinput` checks (send in firefox) (#17820) fix(types): Update title method options type (#17781) fix(deps): update dependency @cypress/request to v2.88.6 🌟 (#17813)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User facing changelog
Additional details
I added the
importsNotUsedAsValuesrule to ourts/tsconfig.json. This enforces than type-only imports be written using the newimport typesyntax. Read about that rule here.The reason this is useful is a lot of the newer ESM based build tools, like Vite, sometimes have problems trying to import types from TypeScript code designed to be used on the server. It will help future proof us.
For example in the unified feature branch ("launchpad"), which is developed using Vite, I may want to import an interface from
server/lib/project-base.ts. That file also imports some Node.js only modules, likefs. If you doimport { ProjectBase } from '@packages/server/lib/project-base.ts, it will error out, since Vite attempts to import the entire file to the browser, including the Node.js-only modules, likefs.`import type { ProjectBase } from '...' works correctly - Vite is able to extract the type, and ignore the actual implementation code. No side effects 😎
Another potential benefit is the TS compiler can likely do some optimizations if it just importing a file for type definitions, not actual runtime code.
There's no real downside to the new
import typesyntax, so I just went ahead and added it to the entire code base. VS Code can even automatically do the correct import for you, since we have the rule intsconfig.json.How has the user experience changed?
N/A
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?