Stream: t-compiler/wg-rls-2.0

Topic: Problems with TypeScript build


matklad (Feb 03 2020 at 11:48, on Zulip):

When I try to build the extension with a task, I get this

matklad (Feb 03 2020 at 11:48, on Zulip):

pasted image

matklad (Feb 03 2020 at 11:48, on Zulip):

Can it be related to that module thing? cc @std::Veetaha ?

std::Veetaha (Feb 03 2020 at 11:52, on Zulip):

@matklad Can you try to set "moduleResoltion": "node" in tsconfig?

std::Veetaha (Feb 03 2020 at 11:57, on Zulip):

Its a miracle how rollup doesn't catch this... It uses some custom TypeScript compilation settings that differ from tsc...

matklad (Feb 03 2020 at 13:04, on Zulip):

Hm, with moduleResolution: node it compiles, but failes at runtime with a parse error

matklad (Feb 03 2020 at 13:05, on Zulip):
rust-analyzer/editors/code/out/main.js:1
(function (exports, require, module, __filename, __dirname) { import * as vscode from 'vscode';
                                                                     ^

SyntaxError: Unexpected token *
std::Veetaha (Feb 03 2020 at 13:11, on Zulip):

Okay, we have very interesting situation) We produce es2015 output for rollup, but when developing this must be a ready-to-use commonjs

matklad (Feb 03 2020 at 13:12, on Zulip):

Am I correct that Node that is used with electorn (12) supports only common js, but not es2015?

std::Veetaha (Feb 03 2020 at 13:13, on Zulip):

Yes, in order to use es2015, files should have extension .mjs.

std::Veetaha (Feb 03 2020 at 13:14, on Zulip):

I propose to create tsconfig.dev.json where it "extends": "./tsconfig.json" and overrides "module": "commonjs"

std::Veetaha (Feb 03 2020 at 13:14, on Zulip):

Otherwise we would need to setup rollup for development watch mode

std::Veetaha (Feb 03 2020 at 13:17, on Zulip):

Or we just downgrade @rollup/typescript-plugin to 2.1.0 and switch to commonjs so I think of the better setup today

matklad (Feb 03 2020 at 13:20, on Zulip):

hm,

https://github.com/rollup/plugins/tree/master/packages/typescript#ignored-options

Ignored options

noEmitHelpers, importHelpers: The tslib helper module always must be used.

matklad (Feb 03 2020 at 13:21, on Zulip):

what does this actually mean?

std::Veetaha (Feb 03 2020 at 13:23, on Zulip):

This is what I saw earlier today, it always uses tslib no matter what importHelpers option is set to.
However it should be enabled to be used for development builds with tsc

matklad (Feb 03 2020 at 13:25, on Zulip):

Hm, I remmeber why tslib ended up in dev deps

matklad (Feb 03 2020 at 13:25, on Zulip):

https://github.com/rollup/rollup-plugin-typescript#installation

matklad (Feb 03 2020 at 13:25, on Zulip):

that's what the docs for rollup originally said

std::Veetaha (Feb 03 2020 at 13:29, on Zulip):

I see that they say that "extensions": [".ts"] is required in commonjs plugin options, maybe this is why the bundling behavior changed from 2.1.0 to 3.0.0
https://github.com/rollup/plugins/blob/master/packages/typescript/README.md#importing-commonjs

std::Veetaha (Feb 03 2020 at 13:32, on Zulip):

Having tslib as dev dependency is inherently weird. Maybe they meant it to be dev dependency because the resulting bundle doesn't depend on anything from node_modules, but this way we could say that any dependency is dev dependency

std::Veetaha (Feb 03 2020 at 13:37, on Zulip):

Let's just downgrade the plugin to 2.1.0 and switch to "module": "commonjs" for now, ok?

std::Veetaha (Feb 03 2020 at 13:41, on Zulip):

Very sorry for wasting you time...

matklad (Feb 03 2020 at 13:53, on Zulip):

I think that uptodate bundler plus a dev dep on tslib is better than old bundler

matklad (Feb 03 2020 at 13:54, on Zulip):

I agree that devdep for tslib is super weired thoguh :)

matklad (Feb 03 2020 at 13:54, on Zulip):

And no, this is definitelly not a time waste, as we've lerned a bunch about the limitation of the tools we use

matklad (Feb 03 2020 at 13:55, on Zulip):

The only waste here is that nodejs ecosystem is kind of ungood, in my humble option, but we just have to work with it

matklad (Feb 03 2020 at 13:56, on Zulip):

And of course I can't place a comment into package.json to explain this weirdness

matklad (Feb 03 2020 at 13:56, on Zulip):

/me shakes fist at heaven

std::Veetaha (Feb 03 2020 at 13:57, on Zulip):

@matklad, I think one day we should migrate from rollup to webpack, since it is the unspoken standard bundler, anyway...
No need for a comment, I'll tackle it today!

Laurențiu Nicola (Feb 03 2020 at 14:04, on Zulip):

There was an attempt to do that in https://github.com/rust-analyzer/rust-analyzer/pull/1451. But isn't rollup supposed to be newer and better?

std::Veetaha (Feb 03 2020 at 14:14, on Zulip):

@Laurențiu Nicola well rollup was published 5 years ago and webpack 8 years ago. I am not sure wether this is the new better guy on the market, I am just following the trends )
https://www.npmtrends.com/rollup-vs-webpack

Laurențiu Nicola (Feb 03 2020 at 14:19, on Zulip):

I just read the blurb, it says "next-generation" :grinning:

Laurențiu Nicola (Feb 03 2020 at 14:22, on Zulip):

I wonder if anyone is working on Code bindings for Rust

std::Veetaha (Feb 03 2020 at 14:31, on Zulip):

Speaking about "next-generation" I heard that initially webpack was a very good piece of software, it got popularity, but somehow its maintainers didn't support it very much and that's why new bundlers (maybe rollup was one of them) appeared. However after some time webpack maintainers caught up and started developing it very much to the state that it is now dominating the market.

matklad (Feb 03 2020 at 14:33, on Zulip):

The metrick I use is the size of package.json.lock :D

Joe Clay (Feb 03 2020 at 15:47, on Zulip):

And of course I can't place a comment into package.json to explain this weirdness

FWIW, the "//": key is reserved for comments in package.json. Feels a bit kludgy to use that though :sweat_smile:

Joe Clay (Feb 03 2020 at 15:50, on Zulip):

and on the topic of webpack vs. rollup, general consensus I've heard is that the former is better for applications, the latter is better for libraries. If you're literally just bundling your JS rather than doing anything fancy, I'd definitely recommend avoiding the hassle of maintaining a Webpack config file :grimacing:

matklad (Feb 03 2020 at 16:43, on Zulip):

@std::Veetaha btw the today's morning problem, "build works via rollup, but not via build task" is exactly an issue which wouldn't be caught by an end-to-end test. It seems like you should just suffer through similar things :-(

std::Veetaha (Feb 03 2020 at 16:46, on Zulip):

No suffering! Build task is just a script that invokes tsc --watch, if we add tsc --noEmit to CI we can tackle at least the issue with compile-time inconsistencies between rollup and tsc.

matklad (Feb 03 2020 at 16:49, on Zulip):

For the discrepancies, I think https://github.com/rust-analyzer/rust-analyzer/pull/3003 might actually be a better approach: just don't let that rollup plugin to mess up with typescript plugin.

But my point is that, for integration, things usually break in a pretty surprising ways, which are not tested. Like, after every specific failure you can add a test that will catch this specific failure, but the probability that it won't catch the next failure is pretty high

std::Veetaha (Feb 03 2020 at 16:51, on Zulip):

Not mess with typescript build

Do you want to feed compiled JavaScript artifacts to rollup instead of using the typescript plugin?

matklad (Feb 03 2020 at 16:52, on Zulip):

exactly

std::Veetaha (Feb 03 2020 at 16:52, on Zulip):

This is a good point, though it may take a bit more seconds to assemble the package, but we won't have these inconsistencies anyway

Last update: Feb 25 2020 at 02:55UTC