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

Errors not reported correctly with incremental build when type is inferred from transitive import #30780

Closed
sheetalkamat opened this issue Apr 5, 2019 · 4 comments · Fixed by #30971
Assignees
Labels
Bug A bug in TypeScript Domain: --incremental The issue relates to incremental compilation Fixed A PR has been merged for this issue

Comments

@sheetalkamat
Copy link
Member

Search Terms:

Code

// tsconfig.json
{
  "compilerOptions": {
    "target": "es5",
    "declaration": true,
    "outDir": "obj",
    "incremental": true
  }
}
// index.ts
import { LazyAction, LazyModule } from './bundling';
const lazyModule = new LazyModule(() =>import('./lazyIndex'));
export const lazyBar = new LazyAction(lazyModule, m => m.bar);

// bundling.ts
export class LazyModule<TModule> {
    constructor(private importCallback: () => Promise<TModule>) {}
}

export class LazyAction<
    TAction extends (...args: any[]) => any,
    TModule
>  {
    constructor(_lazyModule: LazyModule<TModule>, _getter: (module: TModule) => TAction) {
    }
}

// lazyIndex.ts
export { default as bar } from './bar';

// bar.ts
interface RawAction {
    (...args: any[]): Promise<any> | void;
}
interface ActionFactory {
    <T extends RawAction>(target: T): T;
}
declare function foo<U extends any[] = any[]>(): ActionFactory;
export default foo()(function foobar(param: string): void {
});

Expected behavior:
After incremental edit index.d.ts should change to:

//obj\index.d.ts 
import { LazyAction } from './bundling';
export declare const lazyBar: LazyAction<() => void, typeof import("./lazyIndex")>;

Actual behavior:
Run tsc and note obj\index.d.ts is

//obj\index.d.ts 
import { LazyAction } from './bundling';
export declare const lazyBar: LazyAction<(param: string) => void, typeof import("./lazyIndex")>;

Edit bar.ts to remove param for foobar.
Run tsc again and note that index.d.ts doesn't change

Playground Link:

Related Issues:
This is the smaller repro for issue reported by @MLoughry offline

@sheetalkamat sheetalkamat self-assigned this Apr 5, 2019
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Apr 10, 2019
@RyanCavanaugh
Copy link
Member

@sheetalkamat should we fix for 3.5 or can it wait?

@MLoughry
Copy link

Since 3.4.1, this has happened only once in Outlook Web (as far as I know) in the repro I gave @sheetalkamat. So, it’s not a frequent occurrence.

However, it does breed distrust in the build cache, and has caused devs to try clearing the cache and rebuilding when they see an error they don’t immediately recognize.

Obviously, I have no idea how hard this is to fix.

@sheetalkamat sheetalkamat added the Domain: --incremental The issue relates to incremental compilation label Apr 11, 2019
@MLoughry
Copy link

To add: I'm currently working on breaking up our massive project into smaller projects, and I'm enabling strict mode on them one at a time in separate branches. Today I've encountered this kind of issue (stale references) several times when switching branches and/or modifying an upstream project in -b -w mode.

If it would be helpful, I can try to isolate one or more repros of this case, but my assumption is that it's the same underlying issue.

@sheetalkamat
Copy link
Member Author

Yes the issue is we do not generate .d.ts that could change since currently builder tracks potential js emit file change = emitting, Note however we do update semantic diagnostics for the larger set (where we should also emit .d.ts file if enabled) so the fix is relatively simple, just looking at where exactly this should sit)

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: --incremental The issue relates to incremental compilation Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants