Upgrade "is-observable" dependency to v1

[doasync]

Please, upgrade version of is-observable to stable 1.0.0.

is-observable 0.2.0 depends on old symbol-observable: "^0.2.2", redux uses symbol-observable: "^1.0.3", as a result there are multiple versions of symbol-observable in my bundle.

Involved packages:

  "_requiredBy": [
    "/jss-default-unit",
    "/jss-expand",
    "/jss-extend"
  ],

[kof]

How true!

[kof]

Totally forgot about that @appsforartists, I lets send a PR to is-observable package to use a better check, so we can reuse is-observable everywhere.

[appsforartists]

Sorry, I don't have time to mess with this. You can always import jss/utils/isObservable in the mean time.

I did try though: https://github.com/appsforartists/is-observable/tree/esm

No matter what I did, I couldn't get ava to be happy about the import statement in the module version.

~/Projects/is-observable/index.module.js:1
(function (exports, require, module, __filename, __dirname) { import $$observable from 'symbol-observable';
                                                              ^^^^^^

SyntaxError: Unexpected token import

[appsforartists]

(Not to be snarky, but this is precisely why I just used a local copy of isObservable in the first place - PRs take work that isn't always worth it.)

[kof]

He is not transpiling, had to use require probably,

[kof]

Using isObservable from the jss core would make it technically part of the public api)

[kof]

Ok, created a PR https://github.com/sindresorhus/is-observable/pull/5

[appsforartists]

The reason to use the local one isn't because it calls $$observable, it's because it avoids introducing a commonJS dependency into a module-based project. The reason I couldn't open my PR is because I couldn't figure out how to get import statements to work in his setup.

[sindresorhus]

No matter what I did, I couldn't get ava to be happy about the import statement in the module version.

AVA only transpiles test files by default.

it's because it avoids introducing a commonJS dependency into a module-based project.

I don't really see how this is a problem. 99.9% of the npm ecosystem is CommonJS, not ESM.

[doasync]

I thought it would be an easy fix to package.json files:

https://github.com/cssinjs/jss-default-unit/blob/master/package.json#L92 https://github.com/cssinjs/jss-expand/blob/master/package.json#L88 https://github.com/cssinjs/jss-extend/blob/master/package.json#L93

[kof]

introducing a commonJS dependency

I didn't even get to change the build for JSS to have also es modules published. Package is-observable is just one line code, so it is not much harm. Also I really just want to have one source for this work across all JSS packages. An es build can be introduced there later.

[kof]

Turns out is-observer isn't even supporting browsers. It is just for node https://github.com/sindresorhus/is-observable/pull/7#issuecomment-350848945

Lol, such a small issue and so much hustle, following possibilities now:

1. Create a separate package, basically duplicate of is-observable but with essentially this code (https://github.com/sindresorhus/is-observable/pull/7/files)
2. Maintain local utility and make it part of the public interface to be used from plugins
3. Copy paste (needs to be done in every package where used)
4. Change webpack config to not ignore is_observable from node_modules (needs to be done in every package where used)

[kof]

Maybe @appsforartists you want to publish is-observable with proper bundling for es3 and esm?

[appsforartists]

I'm inclined to say JSS plugins should just do

import { isObservable } from 'jss/utils/isObservable'

That code won't run in a browser without bundling, but the rest of JSS won't either. However, it's a super easy patch to completely remove the dependency on is-observable for now.

If somebody really cares about publishing a more flexible isObservable microlib, I'd suggest copying my implementation from https://github.com/material-motion/material-motion-js/blob/develop/packages/core/src/typeGuards.ts. You could pretty trivially use TypeScript to generate .d.ts, ESM, and CommonJS + ES5 targets.

[kof]

Hmm, is it actually fine to require a library which is a peerDependency?

[kof]

Oh man, I totally forgot, the latest version of jss doesn't require plugins to know anything about observables. I remove the observable from styles object and just update the values from subscription. There is no need for exposing isObservable, it can be just an implementation detail of the "observables" internal plugin.

[appsforartists]

which means you should update the official plugins to stop detecting/whitelisting observables?

[kof]

exactly, I am on it, together with the new release

[kof]

Done