solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.47k stars 927 forks source link

Rollup not reading signal types properly #1656

Closed dweng0 closed 1 year ago

dweng0 commented 1 year ago

Describe the bug

Hey guys, I am likely doing someting wrong, but thought it was worth mentioning. So please let me know if i need to pull in a plugin or something. But here it goes:

TLDR: solid-js fails when using rollup to build.

Created a new package using TSDX (Minimum Reproducible code here)

Stripped out the fluff and updated the index.ts file.

import { createSignal } from 'solid-js';

export const Test = () => {
  const [test, setTest] = createSignal(0);

  setTest(1);
  return test();
};

however when the i run npm run build

i get a build failure on this line: setTest(1);

TS2554: Expected 0 arguments, but got 1. with follow up trace giving a throwPluginError

Looks like the call signature created for the setter isn't what the rollup compiler was expecting? Any ways to worko around this?

Your Example Website or App

https://github.com/dweng0/solid-peerjs/tree/rollup-build-failure

Steps to Reproduce the Bug or Issue

  1. Pull down this branch
  2. npm install
  3. npm run build
  4. ???
  5. profit (or loss)

Expected behavior

to build

Screenshots or Videos

Looks like it doesn't quite read image

Platform

Additional context

Error: /solid-peerjs/src/index.ts(6,11): semantic error TS2554: Expected 0 arguments, but got 1. at error (~\node_modules\tsdx\node_modules\rollup\dist\shared\node-entry.js:5400:30) at throwPluginError (~\node_modules\tsdx\node_modules\rollup\dist\shared\node-entry.js:11878:12)
at Object.error (~\node_modules\tsdx\node_modules\rollup\dist\shared\node-entry.js:12912:24) at Object.error (~\node_modules\tsdx\node_modules\rollup\dist\shared\node-entry.js:12081:38) at RollupContext.error (~\node_modules\tsdx\node_modules\rollup-plugin-typescript2\dist\rollup-plugin-typescript2.cjs.js:17237:30) at ~\node_modules\tsdx\node_modules\rollup-plugin-typescript2\dist\rollup-plugin-typescript2.cjs.js:25033:23 at arrayEach (~\node_modules\tsdx\node_modules\rollup-plugin-typescript2\dist\rollup-plugin-typescript2.cjs.js:545:11) at Function.forEach (~\node_modules\tsdx\node_modules\rollup-plugin-typescript2\dist\rollup-plugin-typescript2.cjs.js:9397:14) at printDiagnostics (~\node_modules\tsdx\node_modules\rollup-plugin-typescript2\dist\rollup-plugin-typescript2.cjs.js:25006:12) at Object.transform (~\node_modules\tsdx\node_modules\rollup-plugin-typescript2\dist\rollup-plugin-typescript2.cjs.js:29277:17)

ryansolid commented 1 year ago

Interesting. I use rollup to build all of Solid's libraries and demos but never see this. That being said I usually use babel to do the TS conversion along with our JSX transform and not a seperate plugin.

ryansolid commented 1 year ago

Yeah this isn't a Rollup bug as much as TSDX. It hasn't been touched in a year and looks like it uses stuff from years ago. For typescript isn't even using the official rollup plugin but rollup-plugin-typescript2. I used to use that library back in the day too. I suspect it is just really out of date. Like it uses Rollup 1.32 from 3 years ago.

It's complaining about our setter not taking an argument. Types appear fine here: https://github.com/solidjs/solid/blob/main/packages/solid/src/reactive/signal.ts#L177-L180 Only possibility where it doesn't take an argument if is the generic is undefined and it clearly isn't in this case. I wonder if it doesn't know how to properly handle the tuple return type of createSignal.

But don't see what can be done on our side.

thednp commented 1 year ago

@dweng0 Why use Rollup to build a Solid app when you should just use Vite?

lxsmnsyc commented 1 year ago

@ryansolid we'd usually just recommend tsup (https://github.com/solidjs-community/tsup-preset-solid) or Rollup (https://github.com/solidjs-community/rollup-preset-solid) since TSDX is pretty much abandonware

ryansolid commented 1 year ago

Yeah I guess just close this one as won't fix. Not much to be done here.