Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Allow modular use of emit-less types #28

Closed
2 tasks
Victorystick opened this issue Jan 28, 2016 · 20 comments
Closed
2 tasks

Allow modular use of emit-less types #28

Victorystick opened this issue Jan 28, 2016 · 20 comments
Labels

Comments

@Victorystick
Copy link
Contributor

A regular TypeScript compilation takes many modules into consideration at once. It manages imports and exports of types as well as values, where as ECMAScript is only concerned with values. TypeScript only uses the type information of emit-less types like interface, type, const enum etc. during compilation and discards these from the final emit; they simply won't exist in the output.

This poses a problem since Rollup processes modules in the order they're encountered. Any module transforms have to be done without any information about its dependencies (except for some rare cases with circular dependencies). We simply cannot know if an imported identifier is an emit-less type or a value that is preserved. When Rollup later attempts to hook up the imports of one file with the exports of another, it will complain that the imports for these emit-less types don't have any corresponding exports.

Import and use of a const enum

This means that any import and usage of a const enum defined in another module will fail.

// some-enum.ts
export const enum Foo { bar, baz } // results in the empty string after transpilation

// main.ts
import { Foo } from './some-enum';
  // Rollup -> Error: Module some-enum.ts does not export Foo (imported by main.ts)

console.log( Foo.bar );

What we would want is for the import statement above to be completely removed since no actual value is imported, and for Foo.bar to be replaced with the constant value it corresponds to.

Re-export of an emit-less type

A re-export of a type will fail since the module doing the re-export cannot know that what is being exported doesn't exist except during compilation.

export { Foo } from './some-enum';
  // Rollup -> Error: 'Foo' is not exported by 'some-enum.ts' (imported by main.ts)

In this case, we'd just like the export statement to be removed.

Summary

To be useful in practice the plugin must gracefully handle:

  • imports and usages of const enums defined in other modules
  • re-exports of emit-less types

These problems can only be solved by having TypeScript process several modules simultaneously. Unfortunately, this doesn't fit well with Rollup's current behaviour of processing modules in turn as they're encountered.

Hopefully someone might be able to bring some good insights as to how to solve this problem.

@thorn0
Copy link

thorn0 commented May 10, 2016

The isolatedModules option can probably help here. It's described in this comment.

@DanielRosenwasser
Copy link

Would you not be able to process your input holistically first and then use an emit resolver when processing each individual file?

@Victorystick
Copy link
Contributor Author

@DanielRosenwasser That is an option, which would probably work well for people working fully in TypeScript. But for people working with a combination of languages/Rollup plugins might run into problems if Rollup resolves modules differently from TypeScript.

@jvilk
Copy link

jvilk commented Jun 3, 2016

@Victorystick Can't you compile the input normally, cache the results for each file in-memory, and return those when/if Rollup requests them? Do we expect users to configure Rollup such that it changes module resolution within TypeScript modules in a way that would invalidate that compilation output?

This bug is preventing me from using rollup-plugin-typescript on my project. I will have to invoke the TypeScript compiler first and point rollup to that output on disk. 😦

jvilk pushed a commit to jvilk/BrowserFS that referenced this issue Jun 3, 2016
@AlexGalays
Copy link

Quite a blocker, that issue :(

@ghost
Copy link

ghost commented Jun 24, 2016

@Victorystick 6 months now. Are this issue going to be solved soon?

@ghost ghost mentioned this issue Aug 3, 2016
@ghost
Copy link

ghost commented Aug 11, 2016

@TrySound Are you going to solve this issue as well?

@TrySound
Copy link
Member

@KFlash Yep, some day.

@AlexGalays
Copy link

@TrySound We would love to use rollup with typescript but this bug makes it impossible. Perhaps if you could explain what is involved to produce a fix someone else could take a look ? Is there any change required to rollup core ?

@TrySound
Copy link
Member

@AlexGalays Need to find a way to get plain structure from ts info about types. Keeping an instance of language service is bad solution, because we cannot write it on disk.

@maxdavidson
Copy link
Contributor

@AlexGalays I'm using the tsc command to process my files before piping them through Rollup, instead of using this plugin. I use my plugin rollup-plugin-sourcemaps to inject the source maps generated by tsc into Rollup's transformation chain. It's not ideal, but it works fine and even generates the declarations. My workflow looks something like this.

@btgoodwin
Copy link

@maxdavidson I'm using your method to roll up my libraries (sans external modules, per your readme). The setup is one library requires the other. And in both cases, the process seems to execute fine. When I then try to roll up those libraries with the external libraries into a vendor bundle for an app, the JS file generated by the previous rollup cannot be read by the app rollup. It always throws an "unexepected token" on the "s" in "exports" where the top-level module is exported in the library. Have you come across that when pulling in libraries created using your boilerplate into typescript-based apps?

@maxdavidson
Copy link
Contributor

@btgoodwin I can't seem to reproduce your error. Do you have an example project where this issue occurs? It does however sound more like a problem with your Rollup config. Are you using rollup-plugin-uglify in your vendor bundle? Uglify still doesn't support ES2015 syntax, so it won't together with the es bundle format.

@btgoodwin
Copy link

btgoodwin commented Oct 7, 2016

@maxdavidson I'm not using uglify (and probably won't at this point). Also to be clear, your example has provided me a great education that I've hardly been able to mine out of the internet all day. So thank you; I do not think there is a problem with your example. At this point I'm not fully trusting myself (I'm new to these tools/processes).

For my application-level rollup, I was allowing it to consume the UMD bundles for my libraries which was causing it to throw that unexpected token on "event_s_". I switched to the alias plugin as you and others show for the rxjs-es library, and pointed to the ES2015 modules of my libraries. That seems to work (or at least, it doesn't throw any errors).

The gist is I'm trying to get my head around how I transition from what I had before, a big ugly SystemJS config that required lengthy import statements in the typescript files, to something more clean like the @angular/core module. And at this point it's clearer to me that I'm probably misunderstanding how to setup my libraries for as easy development as deployment.

Edit: Perfect example of my lack-of-a-clue is that roll-up is complaining about external modules (like @angular/core, etc.), so I add them to the external list. Then it complains that no name was provided for the external module. I see that the @angular/core's rollup declares some names for its dependencies on RxJS, which for me also clears off those messages for Observable and Subject. However doing the same for my library's dependencies on operator match, catch, or any of the angular modules doesn't resolve the warnings in those cases. I'm left with not wanting to bundle external dependencies into my libraries, but also not having a way to get rid of these warnings while understanding if I've actually corrected its complaint (judging by the issues list for rollup on this question in particular, it seems to have no clear, explained resolution).

@StevenDoesStuffs
Copy link

Current status?

@unional
Copy link

unional commented Jan 12, 2017

@maxdavidson Thanks! Your boilerplate just rock! 🌷

@jahan-addison
Copy link

Not sure how this is viable without emitting type errors..

@AlexGalays
Copy link

@jahan-addison I think this project was just a proof of concept.

sebdurand added a commit to Orange-OpenSource/OCast-JS that referenced this issue Jul 19, 2018
sebdurand added a commit to Orange-OpenSource/OCast-JS that referenced this issue Jul 19, 2018
* fix rollup export interface @see rollup/rollup-plugin-typescript#28

* fix attach/detach listeners after load/clear

* fix return id tracks not implemented

* fix attach/detach listeners after load/clear

* fix return id tracks not implemented

* Update package.json

* Update package.json

* add skip_cleanup: true / deploy / npm
sebdurand added a commit to Orange-OpenSource/OCast-JS that referenced this issue Jul 19, 2018
* 1.0.3 (#3)

* refactoring : playbackstatus as int

* fix reply connection status

* fix getMetadata

* fix tracks

* fix format code

* 1.0.4 (#12)

* refactoring : playbackstatus as int

* fix reply connection status

* fix getMetadata

* fix tracks

* fix format code

* Fix playback status, connection status, metadata and tracks support.

* refactoring : playbackstatus as int

* fix reply connection status

* fix getMetadata

* fix tracks

* fix format code

* fix npmjs deploy

* fix npm deploy

* fixes #6 #7 #8 #9 #13  (#18)

* fix rollup export interface @see rollup/rollup-plugin-typescript#28

* fix attach/detach listeners after load/clear

* fix return id tracks not implemented

* fix attach/detach listeners after load/clear

* fix return id tracks not implemented

* add skip_cleanup: true / deploy / npm

* update protocol

* fix handle propertly transmission errors
@bodograumann
Copy link

The re-export is the same issue as in webpack afaics. The solution is also the same. Instead of

export { MyInterface } from "./my-module";

use

import { MyInterface } from "./my-module";
export type MyInterface = MyInterface;

@shellscape
Copy link
Contributor

Hey folks (this is a canned reply, but we mean it!). Thanks to everyone who participated in this issue. We're getting ready to move this plugin to a new home at https://github.com/rollup/plugins, and we have to do some spring cleaning of the issues to make that happen. We're going to close this one, but it doesn't mean that it's not still valid. We've got some time yet before the move while we resolve pending Pull Requests, so if this issue is still relevant, please @ me and I'll make sure it gets transferred to the new repo. 🍺

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests