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

Why are interface declarations removed? #24

Closed
onigoetz opened this issue Jul 19, 2017 · 20 comments · Fixed by #406
Closed

Why are interface declarations removed? #24

onigoetz opened this issue Jul 19, 2017 · 20 comments · Fixed by #406
Assignees
Labels
kind: bug Something isn't working properly solution: duplicate This issue or pull request already exists solution: workaround available There is a workaround available for this issue topic: type-only / emit-less imports Related to importing type-only files that will not be emitted

Comments

@onigoetz
Copy link

Hi,

I'm trying to move from webpack to rollup for a library and I have trouble getting declarations to work correctly.

It seems that interfaces declarations aren't exported, this is a problem when I have things like :

import { RenderedCell } from "../RenderedCell";

export default class MergedCell implements RenderedCell {
    // ...
}

I get the error that RenderedCell can't be found.
I worked in webpack, and I can't understand what's the difference in my configuration that broke this. but I seem to understand that it's normal with Rollup.

BTW. I'm using the latest rollup, rollup-plugin-typescript2 and rollup-plugin-uglify I can post my configurations if needed

@ezolenko
Copy link
Owner

Is this a runtime error, or typescript error? Could you post how you export the interface from RenderedCell itself?

@ezolenko ezolenko self-assigned this Jul 19, 2017
@onigoetz
Copy link
Author

The thing is that the file isn't generated at all, and it fails when using the library in another project.

@ezolenko
Copy link
Owner

ezolenko commented Jul 20, 2017

Wait, are you expecting RenderedCell interface to be generated? From the code you posted typescript would expect file named RenderedCell.ts to exist one directory above current file and it would need to contain something like this:

export interface RenderedCell 
{ 
    // ...
}

Could you give more details on your project structure? (file tree, tsconfig, rollup config, etc)

@onigoetz
Copy link
Author

yes I'm expecting it to be generated.

Here are my configurations :

running the build : rollup -c -f es -n sq-web-component-table -o dist/sq-web-component-table.es.min.js

I didn't put the whole tree because it's a quite big library and it's more noise than signal. But you can already see that many files aren't generated in the dist.

rollup.config.ts
import typescript from 'rollup-plugin-typescript2';
import uglify from 'rollup-plugin-uglify';
import { minify } from 'uglify-es';
 
export default {
    entry: './src/index.ts',
    dest: 'dist/sq-web-component-table.min.js',
    sourceMap: true,
 
    plugins: [
        typescript(),
        uglify({}, minify)
    ]
}
tsconfig.json
{
  "compilerOptions": {
    "target": "es5",
    "module": "es2015",
    "moduleResolution": "node",
    "declaration": true,
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "outDir": "dist"
  },
  "include": [
    "src/**/*.ts"
  ],
  "exclude": [
    "dist",
    "node_modules",
    "**/*-test.ts"
  ]
}
RenderedCell.ts
import {RenderedElement} from "./RenderedElement";
import {RenderedRow} from "./RenderedRow";
export interface RenderedCell extends RenderedElement {
    isMergedCell(): boolean;
    getColspan(): number;
    setColspan(colspan: number): void;
    getParent(): RenderedRow;
}
File tree
.
├── src
│   ├── index.ts
│   └── table
│       ├── extensions
│       │   ├── base
│       │   │   ├── FirstTableExtension.ts
│       │   │   ├── LastTableExtension.ts
│       │   │   ├── SectionDispatcher.ts
│       │   │   ├── SectionManager.ts
│       │   │   └── SizeChangeMonitor.ts
│       │   ├── InvalidateEvent.ts
│       │   ├── InvalidateSizesEvent.ts
│       │   ├── RenderedCell.ts
│       │   ├── RenderedElement.ts
│       │   ├── RenderedRow.ts
│       │   ├── RenderedSection.ts
│       │   ├── RenderedTable.ts
│       │   ├── SectionClickedEvent.ts
│       │   ├── support
│       │   │   └── ...
│       │   ├── TableAttributes.ts
│       │   ├── TableExtensionChain.ts
│       │   ├── TableExtensionList.ts
│       │   └── TableExtension.ts
│       ├── model
│       │   └── ...
│       ├── TableRenderingOptions.ts
│       ├── Table.ts
│       ├── util
│       │   └── ...
│       └── view
│           └── ...
├── dist
│   ├── index.d.ts
│   ├── sq-web-component-table.es.min.js
│   ├── sq-web-component-table.es.min.js.map
│   ├── sq-web-component-table.min.js
│   ├── sq-web-component-table.min.js.map
│   └── table
│       ├── extensions
│       │   ├── base
│       │   │   ├── FirstTableExtension.d.ts
│       │   │   ├── LastTableExtension.d.ts
│       │   │   ├── SectionDispatcher.d.ts
│       │   │   ├── SectionManager.d.ts
│       │   │   └── SizeChangeMonitor.d.ts
│       │   ├── InvalidateEvent.d.ts
│       │   ├── InvalidateSizesEvent.d.ts
│       │   ├── RenderedRow.d.ts
│       │   ├── SectionClickedEvent.d.ts
│       │   ├── support
│       │   │   └── ...
│       │   ├── TableAttributes.d.ts
│       │   └── TableExtensionList.d.ts
│       ├── model
│       │   └── ...
│       ├── Table.d.ts
│       ├── util
│       │   └── ...
│       └── view
│           └── ...
├── node_modules
│   └── ...
├── __tests__
│   └── ...
├── package.json
├── rollup.config.js
└── tsconfig.json

@ezolenko
Copy link
Owner

ezolenko commented Jul 21, 2017

Ah, I see. Is the lib opensource by any chance?

Try building with those options:

typescript({ verbosity: 2, clean: true }),

Check if all files you expect are being processed. If declarations suddenly start being generated then there is a cache problem somewhere.

If RenderedCell.ts is not mentioned in output (rpt2: transpiling '...'), increase verbosity to 3 and check if files that are supposed to import it are doing that. You should see something like

rpt2: resolving '../RenderedCell'
rpt2:     to '...'

If the problem is not obvious then, I'll add more logging somewhere.

@onigoetz
Copy link
Author

I would love to, might be in the future :)

Thanks for the tricks, I will try this on monday.

@wessberg
Copy link
Contributor

If you set declaration: false in the compilerOptions section of your tsconfig and recompile, does the issue go away? If yes, is the error reported by TSC related to one of the files within the dist directory? If yes is your answer to both, does the error go away if you introduce some non-type logic to the file containing RenderedCell file such as an export of a simple function? If yes, then the reason is that the file containing RenderedCell is considered unused and Typescript won't ever prepare declarations for it.

If all of this applies to you, I see 2 options:

  • Stop generating declarations for your bundle.
  • Make sure that roll-up won't tree-shake the file away entirely by introducing changes to the file that isn't type related,. Yes, it's a hack and I hope we find a way to do better

@onigoetz
Copy link
Author

Hi,

sorry for the long delay, I tried your tips and tricks but I still couldn't get the type definitions for interfaces generated.

I changed the configuration to typescript({ verbosity: 3, clean: true }).
In the output of the generated build there is not a single mention of the RenderedCell interface.

I'm 100% sure this file is needed, as the index.d.ts of the library directly requires it and all files but the interfaces are generated, but the interfaces are still in the imports of the index.d.ts file.

I will dive in the code to see if there is a particular reason why those files aren't generated.

@wessberg Stop generating declarations for my bundle is not an option as this is a library that is part of a bigger project all written in TypeScript, so if it doesn't have typings it doesn't work.

@onigoetz
Copy link
Author

So, I made some investigation and created a repository that reproduces my case with a minimal set of configuration : https://github.com/onigoetz/typescript-rollup-experiment

I also included a webpack configuration to compare how webpack makes it work.

My understanding of the problem is that when rollup-plugin-typescript2 requires the compiled version of a file (let's say index.ts) Typescript will return the compiled files and it's imports, but without the interfaces in the imports.

That's why rollup never generates them : it's not aware of their existence.

To run the repository do an npm install && npm run build inside the directory.

@wessberg
Copy link
Contributor

If you add:
import "./SomeInterface"; (exactly like that, - without any named bindings) to your index.ts file, it should work. Does it?

Unfortunately, Rollup doesn't invoke the transform handler of plugins for declaration-only files such as files containing only type declarations. The thing above should work since that forces Rollup to evaluate the file. This issue: rollup/rollup-plugin-typescript#28 explains how and why this issue exists. Theoretically, we could manually resolve all dependencies of your application from the entry point and determine those that only contain type declarations, but there is no way of forcing rollup to invoke the transform hook (beside other obvious downsides such as much added build-step complexity).

I'm afraid we can't do much about this without altering rollup itself. The "hack" I described should work for now though.

@ezolenko
Copy link
Owner

Yep, just checked, adding import "./SomeInterface"; in the test repo fixes things.

When importing types only, typescript elides import statements from js output and rollup doesn't see those files and thus doesn't send them for transpiling.

I'll try to cheat and monitor which files typescript requested from disk vs which files rollup sent for transpiling. Should be possible to force-transpile the difference with definitions only flag. This might result in too many d.ts files in some cases I though (for things rollup has legitimately shaken from the tree for example)

@onigoetz
Copy link
Author

Thanks for looking into it, I will also check if I can make a contribution for this.

ezolenko added a commit that referenced this issue Aug 15, 2017
@ezolenko
Copy link
Owner

@onigoetz, ok, try your project with master. Check if right declarations are generated, but also check that no unexpected ones are :)

@onigoetz
Copy link
Author

Awesome, you litteraly fixed it while I was sleeping :)

I tested it and it almost works fine.

There is just a small issue, but I think it's something we can't do anything against, the webpack loader does exactly the same and the file list is generated by TypeScript itself.

As an example; If I have three files in my repo :

  • index.ts importing SomeInterface.ts
  • SomeInterface.ts
  • nongenerated.ts importing SomeInterface.ts

nongenerated.ts is never referenced but its types are generated anyway.

@ezolenko
Copy link
Owner

Cool, thanks for testing. I'll release it a bit later after another issue is confirmed.

Yeah not much we can do when bypassing rollup. You should be able to exclude those files manually in exclude option, but that's a poor workaround.

@ezolenko
Copy link
Owner

ezolenko commented Aug 23, 2017

Ok, in 0.5.1 now

@six006
Copy link

six006 commented Aug 20, 2018

work well, just configure tsconfig.json

{
    "compilerOptions": {
        "module": "es2015",
        "target": "es5",
        "declaration":true,
        "declarationDir": "lib",
        "typeRoots": [
            "node_modules/@six006",
            "node_modules/@types"
        ],
        "lib": [
            "es2015",
            "es2017",
            "es5"
        ],
    },
    "include": [
        "src/index.ts",
        "src/module/**/*.ts"
    ],
    "exclude": [
        "node_modules"
    ]
}

include section tell you what to generate

@JoshMcCullough
Copy link

JoshMcCullough commented Jan 25, 2019

YMMV but I got it to work like this ...

before

export type MyType = { ... };

after

type MyType = { ... };

export { MyType }

@yiochen
Copy link

yiochen commented Jan 31, 2019

For what it's worth, I just call tsc --emitDeclarationOnly after rollup, which overrides the declaration files into build folder.
in my package.json

"scripts": {
    "build": "rollup -c",
    "postbuild": "tsc --emitDeclarationOnly"
}

@agilgur5
Copy link
Collaborator

agilgur5 commented May 25, 2022

See #211 for the root cause of this. The workarounds here provide a way of getting past this in many situations, but not quite all of them. Resolving #211 (and related type-only issues) should fix this class of bug entirely.

@agilgur5 agilgur5 added the solution: duplicate This issue or pull request already exists label May 25, 2022
Repository owner locked as resolved and limited conversation to collaborators May 25, 2022
@agilgur5 agilgur5 added the kind: bug Something isn't working properly label May 25, 2022
@agilgur5 agilgur5 changed the title Why are interface declarations removed ? Why are interface declarations removed? Jun 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind: bug Something isn't working properly solution: duplicate This issue or pull request already exists solution: workaround available There is a workaround available for this issue topic: type-only / emit-less imports Related to importing type-only files that will not be emitted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants