thi-ng / umbrella

⛱ Broadly scoped ecosystem & mono-repository of 198 TypeScript projects (and ~175 examples) for general purpose, functional, data driven development
https://thi.ng
Apache License 2.0
3.31k stars 144 forks source link

[rstream] error when using skypack build #462

Closed snosenzo closed 3 months ago

snosenzo commented 3 months ago

Getting this error when trying to use stream or reactive in the skypack build:

Uncaught ReferenceError: Must call super constructor in derived class before accessing 'this' or returning from derived constructor
at new Stream (stream.js:35:5)

Example jsfiddle here: https://jsfiddle.net/snosenzo/qnub2acg/91/

I think it's an issue with the build because the source code around the error looks like:

image

which would mean it is indeed using this before calling super.

Maybe it's a jsfiddle issue? Open to hear if I'm doing anything incorrect.

postspectacular commented 3 months ago

Hi @snosenzo - thank you for reporting this! It indeed seems to be a transpilation issue with skypack. The JS emitted by TypeScript (and the code distributed in the package) currently should look like this:

class Stream extends Subscription {
  src;
  _cancel;
  _inited;
  constructor(src, opts) {
    const [_src, _opts] = isFunction(src) ? [src, opts || {}] : [void 0, src || {}];
    super(
      _opts.error ? { error: _opts.error } : void 0,
      __optsWithID("stream", _opts)
    );
    this.src = _src;
    this._inited = false;
  }

Will have to look into this, since this likely will impact alot of other packages... Do you know if that's only a recent issue or is it your first time trying out the skypack version?

postspectacular commented 3 months ago

Yeah, looking at https://cdn.skypack.dev/@thi.ng/rstream/stream, it seems skypack defaults to ES2019 syntax and hence will transpile & inject the class field initializers:

/*
 * Skypack CDN - @thi.ng/rstream@8.4.0
 *
 * Learn more:
 *   📙 Package Documentation: https://www.skypack.dev/view/@thi.ng/rstream
 *   📘 Skypack Documentation: https://www.skypack.dev/docs
 *
 * Pinned URL: (Optimized for Production)
 *   ▶️ Normal: https://cdn.skypack.dev/pin/@thi.ng/rstream@v8.4.0-jseNr7pQpInuIMDkQFZ2/mode=imports/optimized/@thi.ng/rstream/stream.js
 *   ⏩ Minified: https://cdn.skypack.dev/pin/@thi.ng/rstream@v8.4.0-jseNr7pQpInuIMDkQFZ2/mode=imports,min/optimized/@thi.ng/rstream/stream.js
 *
 */

// Browser-Optimized Imports (Don't directly import the URLs below in your application!)
export * from '/-/@thi.ng/rstream@v8.4.0-jseNr7pQpInuIMDkQFZ2/dist=es2019,mode=imports/optimized/@thi.ng/rstream/stream.js';
export {default} from '/-/@thi.ng/rstream@v8.4.0-jseNr7pQpInuIMDkQFZ2/dist=es2019,mode=imports/optimized/@thi.ng/rstream/stream.js';

I tried manually adding ?dist=es2022 but that doesn't seem to have any effect. Need to do more reading...

postspectacular commented 3 months ago

@snosenzo Also hmmm.... this might explain the issue: https://github.com/skypackjs/skypack-cdn/issues/365 - looks like this whole CDN project is possibly abandoned (and hence maybe also no ES2022 support???), but no official word to be found and I'm personally not really using it (so didn't notice so far....)

postspectacular commented 3 months ago

@snosenzo I think I will switch all docs to use https://www.jsdelivr.com. So you just need to replace:

https://cdn.skypack.dev/@thi.ng/rstream

...with:

https://esm.run/@thi.ng/rstream

(A quick look at the returned code there, it seems they thankfully don't mess around with transpilation to older syntax, at least not in the above way...)

snosenzo commented 3 months ago

@postspectacular Thanks so much for looking into this! Looks like the esm.run links do indeed work as a drop-in replacement :D

Will have to look into this, since this likely will impact alot of other packages... Do you know if that's only a recent issue or is it your first time trying out the skypack version?

Also yeah, first time trying to use the skypack version.