Skip to content

Side-effect imports are checked if file uses JSX: unintentional breaking change? #28538

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

Closed
mattmccutchen opened this issue Nov 15, 2018 · 2 comments
Assignees
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Domain: JSX/TSX Relates to the JSX parser and emitter High Priority

Comments

@mattmccutchen
Copy link
Contributor

mattmccutchen commented Nov 15, 2018

Commit 7a71887 (#27340) appears to have introduced an unintentional breaking change: side-effect module imports are now checked, but only if the file uses JSX. This is currently causing the DefinitelyTyped CI (which uses typescript@next) to fail on @types/react-table.

TypeScript Version: 7a71887 (problem does not occur in 12e371b)

Search Terms: side effect module import jsx

Code

// code.tsx
import * as React from "react";
import "nope";
<div/>;
// tsconfig.json
{
    "compilerOptions": {
        "jsx": "react",
        "strict": true
    }
}
// package.json
{
  "devDependencies": {
    "@types/react": "^16.7.6"
  }
}

Expected behavior: No compile error.

Actual behavior: error TS2307: Cannot find module 'nope'. But if the JSX element is removed from the file, the error goes away.

Playground Link: N/A, React dependency

Related Issues: #28032, etc. for the previous behavior

@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.2.1 milestone Nov 15, 2018
@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Domain: Declaration Emit The issue relates to the emission of d.ts files High Priority labels Nov 15, 2018
@weswigham
Copy link
Member

I called it out when it surfaced in our user suite this morning here. I wanted to double check that we didn't want to check them because TBH it's strange we don't - the only reason we didn't is because by chance we never attempted to resolve the export shape for the associated module and since, ofc, nobody asked for any (there aren't any requested, syntactically) the resolution never got checked.

@weswigham
Copy link
Member

(Also it's not tied to jsx - anything that triggers a symbol visibility check on something in the containing file is the prerequisite - so declaration emit being on aughta do it, too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Declaration Emit The issue relates to the emission of d.ts files Domain: JSX/TSX Relates to the JSX parser and emitter High Priority
Projects
None yet
Development

No branches or pull requests

3 participants