staltz / xstream

An extremely intuitive, small, and fast functional reactive stream library for JavaScript
http://staltz.github.io/xstream/
MIT License
2.37k stars 137 forks source link

Use export default xs to enable autoimport in VSCode #242

Closed Nitive closed 6 years ago

Nitive commented 6 years ago

vscode-xs-import-demo

staltz commented 6 years ago

Nice! Thanks

ryota-ka commented 6 years ago

This change possibly breaks the existing code. Before as PR, the default export of the module was Stream as both of its value and its type, but since v1.13.0 only its value is exported.

This is a breaking change for the code which utilises the default-exported stuff as Stream type (not only as a value).

For example, I looked into lib/ directory of staltz/cycle-onionify, to find the following code in pickMerge.d.ts.

import xs from 'xstream';
import { InternalInstances } from './types';
export declare function pickMerge(selector: string): (inst$: xs<InternalInstances<any>>) => xs<any>;

This doesn't compile now as xs cannot use as Stream type.

staltz commented 6 years ago

Very good point @ryota-ka I hadn't intended xs<T> to be a valid use of xstream, but since enough people used that, it's a breaking change for them.

staltz commented 6 years ago

That said, it's perceivable only for TypeScript users who directly use it. Those who indirectly use it (like in onionify lib/pickMerge.d.ts) can reinstall/recompile and no error would happen.

ryota-ka commented 6 years ago

@staltz Then any plans to release a new version of cycle-onionify with recompiled lib/*.d.ts? A project relying on it reports errors as below and doesn't compile.

node_modules/cycle-onionify/lib/pickCombine.d.ts(3,64): error TS2304: Cannot find name 'xs'.
node_modules/cycle-onionify/lib/pickCombine.d.ts(3,95): error TS2304: Cannot find name 'xs'.
node_modules/cycle-onionify/lib/pickMerge.d.ts(3,62): error TS2304: Cannot find name 'xs'.
node_modules/cycle-onionify/lib/pickMerge.d.ts(3,93): error TS2304: Cannot find name 'xs'.
staltz commented 6 years ago

Thanks! I released onionify v5.1.0 can you test it?

ryota-ka commented 6 years ago

@staltz Thank you for your quick response and action. I tested the new version and everything seems perfect now 🎉

Nitive commented 6 years ago

The breaking change can be fixed by creating a type along with a constant

  const xs = Stream;
+ type xs<T> = Stream<T>;
  export default xs;

@staltz I can create a PR if you like. I think it's better to fix breaking change now and perhaps remove type export in the next major release

staltz commented 6 years ago

@Nitive That's a wise idea. Let's do that, the PR would be welcomed.