fix typescript config for rollup

[cellog]

It turns out that even with the nodeResolve fixes, we still need the typescript plugin to build with rollup. This PR fixes that

[nickserv]

Is this needed to fix the Babel type issue you mentioned? If so, I think we should go with your original approach and remove both nodeResolve and Babel (or at least the Babel Typescript preset). Otherwise we're going to have two tools compiling TypeScript syntax, which doesn't feel right to me.

[cellog]

We need babel for sure. It just removes all typescript syntax and converts to Javascript. We use tsc independently to verify types in the build step. I suppose there must be some other way to do this that is better?

To answer your question directly, when I ran the branch I'm about to draft PR, it failed on the first TS syntax. Maybe you can grab that branch and play around with it to find the best config? You seem more versed in the options than I am, it would be good to get this just right.

[cellog]

(#3520 is the draft PR @nickmccurdy)

[nickserv]

The TypeScript compiler (tsc) can generate JS, it's not just a typechecker. If we disable noEmit, the TypeScript compiler will emit JS, and we can either remove Babel or just remove the TypeScript Babel preset if we need to use JS features not supported in TS (unlikely, even JSX works in TS).

Thanks, I'll take a look at #3520 and let you know if I have better ideas. Sorry if my previous config PRs caused confusion, I'm less experienced with the Babel TypeScript preset so I'm not as aware of its limitations compared to the TypeScript compiler.

[nickserv]

I see what you mean, removing the TypeScript Rollup plugin causes unexpected token errors, but I'm not sure why (note that you have to disable JSX in tsconfig to this this first, or TS will think that the <> type assertion syntax is JSX). Meanwhile giving the extension list to Babel causes a circular dependency for some reason.

[nickserv]

I managed to fix the syntax error, but there's a circular dependency in #3520. If you want to use the TypeScript compiler, you cannot import the declaration file from src/index.ts, because it has the same path, which will cause the index file to import itself and crash with a stack overflow. It seems like the best way to fix this would be to move index.d.ts to src/types.ts and import that in src/index.ts without importing src/index.ts, ., or .. in src/types.ts.

[nickserv]

Also I'm still confused about how the Babel issue was solved. Is the TypeScript compiler needed for this or can we remove it in favor of Babel?

[cellog]

ok, @nickmccurdy @timdorr I'm pretty sure this is the simplest working setup.

I tried changing whether babel had its extensions, and removing the nodeResolve. This fixed nothing. We need rollup-plugin-typescript2.

I think this is the best setup for now. Perhaps we'll find something better later

[nickserv]

Thanks for your help, I know this isn't easy. In the case that we need TypeScript, I'd rather remove Babel, as we don't seem to be using any part of it that TypeScript itself can't handle. Would you like me to make a PR trying that? We could merge this for now, assuming it works with your other PR.

[cellog]

not at all, I enjoy this stuff :)

I prefer to keep both, as babel has much better support for transpiling features down. typescript is compiling to the latest javascript, and then babel refines it for us. I also imagine that in the future, support for ts will improve, and we can revisit it then.

The final call is @timdorr in this, my opinions are only opinions

[cellog]

OK. I finally have some clarity on this.

The configuration will go through 2 stages.

1. While we still have an index.d.ts, the config in this PR will be used.
2. Once all the types are moved out, we can use a config that is more standard, and remove the typescript plugin.

So, this PR should be considered a step-along-the-path PR, but an essential one.

[nickserv]

Interesting, so our dependency on the TypeScript compiler is only because of how we're migrating the index.d.ts file? In that case I'm ok with merging this, but I'd prefer to switch to either only Babel or only TypeScript once it's fully migrated.

[cellog]

Interesting, so our dependency on the TypeScript compiler is only because of how we're migrating the index.d.ts file? In that case I'm ok with merging this, but I'd prefer to switch to either only Babel or only TypeScript once it's fully migrated.

This is explicitly the plan I'd like to take.

My motivation is having human-readable diffs along the way, and a working config as we go. If this seems too git-focused, I'm OK doing everything-at-once, but it should make it easier to roll back if I/we make a mistake along the way. At least that's my theory :P

[cellog]

so... anything need changing in this PR?

[timdorr]

Sorry, I've been watching from a distance. Also, I've had family in town, so I haven't had time for OSS stuff in the evenings.

Let's get this in for now and then get it right back out :)

[cellog]

no worries, thanks for taking a look!