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

[WIP] Progress towards 2.0 support and tslib #91

Closed
wants to merge 19 commits into from
Closed

Conversation

PaulBGD
Copy link
Collaborator

@PaulBGD PaulBGD commented May 23, 2017

This was referenced May 23, 2017
@LukeSheard
Copy link

I've just started to use this and noticed the lack of TS2 support. What else needs doing to do this?

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented May 23, 2017

@LukeSheard With this PR TS2 support is included and works. Mostly working on other issues now.

@LukeSheard
Copy link

@PaulBGD Lemme know if you want a hand. Would be happy to contribute.

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented May 23, 2017

@LukeSheard Would love some more tests of the tslib helpers and I need to figure out a way to test that the library supports both 1.0 and 2.0.

The biggest thing I think is integrating #84, but I'm waiting on a response from @kryops on that.

@LukeSheard
Copy link

@PaulBGD I've just tried out https://github.com/ezolenko/rollup-plugin-typescript2 as well and it's pretty slick. We could probably take a lot of ideas from that.

@ezolenko
Copy link

Main difference of this plugin (after ongoing updates) and mine is error checking -- all the caching and other stuff is mainly for solving problems introduced by error checking. I'm also probably committing a few grave sins as far as using plugin API goes.

Feel free to take as much or as little as makes sense. :) Once official plugin starts supporting error checking (#43), I'll depreciate mine (unless it is faster :)).

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented May 23, 2017

The only thing left before I'm ready to publish this, is that we need way more tests. All the issues I listed seem to be complete with the changes in this branch.

EDIT: also will be looking at performance, to see if there's anything we can do to improve that.

@matthew-dean
Copy link

Btw, typescript should be listed in package.json as a peer dependency, not a dependency IMO. That would better address issues with typescript version and remove the need to explicitly pass in TypeScript.

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented Jun 2, 2017

@matthew-dean The issue is, we want to provide a single install way for people to get started with Rollup and TypeScript. While I agree that a lot of people that use this plugin will just install their own TypeScript version, I think including this single package will make this easier for newer users.

@goumang2010
Copy link

Is there any progress?

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented Jun 28, 2017

@goumang2010 I'm currently trying to find the best way to support definition files, this seems to fulfill the same behavior as tsc but feels like a hack: https://github.com/rollup/rollup-plugin-typescript/pull/91/files#diff-1fdf421c05c1140f6d71444ea2b27638R92

@YowaiCoder
Copy link

waiting for this, I'm not considering non-official plugins or gulp-ts or whatever.

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented Jul 1, 2017

@fightingcat Something you could help with, check if this PR just works. Ideally it should work the same or better with the current master + typescript 2.

@StreetStrider
Copy link

@PaulBGD this PR is intended to introduce tslib? Is it correct that tslib is a natural replacement for now present «synthetic» typescript-helpers?

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented Sep 17, 2017

@StreetStrider Yes.

@Rich-Harris
Copy link
Contributor

I've just updated this branch so that it's mergeable, but none of the tests will pass — it's giving me this error...

Cannot find type definition file for 'react'.

...even for tests that have nothing to do with React. Any idea what's going on?

@StreetStrider
Copy link

@Rich-Harris, just checked this, I'm seeing the same thing. @types/react is present on my side (I believe, you have one too). That strange, because when I use this plugin it does not fail on any typecheck errors. I use tsc for typecheck (--noEmit) and Rollup + plugin for bundling/transpiling. Rollup and plugin do all the work even if any typecheck errors is present. So this error presumably have another nature.
@PaulBGD

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented Sep 24, 2017

@Rich-Harris Yeah I updated it locally and ran into the same issue, the main thing I can figure out is that we're discovering .ts files wrong (or not letting typescript handle discovery..)

@niklashigi
Copy link

niklashigi commented Dec 23, 2017

I'm currently trying to find the best way to support definition files

@PaulBGD Any updates on this? Being able to generate .d.ts files for Rollup bundles would be incredibly useful.

@PaulBGD
Copy link
Collaborator Author

PaulBGD commented Dec 25, 2017

Sorry when I took this on I thought I had more time to work on it. I can try to spend some more time on it, but I'm definitely looking towards merging one of the better working forks back upstream.

@grahammendick
Copy link

There's a workaround for the tslib issues

  1. Install tslib and rollup-plugin-node-resolve, npm install tslib rollup-plugin-node-resolve
  2. Tell the typescript plugin to import tslib, set importHelpers to true.
  3. Use the node resolve plugin to generate the helper code from the tslib package
plugins: [
    rollupTypescript({
        importHelpers: true,
    }),
    nodeResolve({ jsnext: true, main: true })
]

@shellscape
Copy link
Contributor

shellscape commented Nov 24, 2019

@PaulBGD we're getting ready to move this plugin to a new home at https://github.com/rollup/plugins, and unfortunately we don't have time as a small team to review all pending PRs before we do that. We're closing any pending feature PRs, but please do not delete your branch. We'd love for you to open a new PR for review at the new repo once that move is done. There shouldn't be too many changes, just a new file structure, so it should be relatively painless.

@shellscape shellscape closed this Nov 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.