promesa - missing functions

[awb99]

This three functions that get value from a promise do not work :

(p/extract r)   ;  not in namespace promesa.core
(deref r)
@r

This 3 functions that get the status of a promise are not included in the promesa.core namespace:

(p/pending? r)
(p/resolved? r)
(p/done? r)

[borkdude]

The missing functions can be added by upgrading promesa, but at the same time upgrading promesa will break existing functionality since stuff has been removed and renamed between version 9 and 10.

It's a pity that promesa works with breaking changes. Now in every project that I've exposed promesa I also have to sell these breaking changes to my users. I like promesa, but I'm starting to regret choosing this lib in this regard to be honest. It goes against the ethos of the Clojure ecosystem to prevent breaking changes, only accrete new stuff or rename your namespace/project. @niwinz

[niwinz]

Can we elaborate on this? I mean promesa has breaking changes, but they are mainly on internal API, and some on the recently one added.

In any case no one is opened an issue about any breakage... Many of them are because i have no hint about concrete usage.

If you have some issue with public api and want to preserve some functionality I inivite you all to please, open issue and lets talk, we are all humans and can reason about that.

If some previous names need to be preserved, I'm open to talk on it and restore if it is possible.

El lun., 14 ago. 2023 17:16, Michiel Borkent @.***> escribió:

The missing functions can be added by upgrading promesa, but at the same time upgrading promesa will break existing functionality since stuff has been removed and renamed between version 9 and 10.

— Reply to this email directly, view it on GitHub https://github.com/babashka/sci.configs/issues/21#issuecomment-1677529378, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGN7KISIQ36WR7RUUFBL2TXVI6NJANCNFSM6AAAAAA3PZZMP4 . You are receiving this because you were mentioned.Message ID: @.***>

[borkdude]

@niwinz OK, let's elaborate one bit at a time, thank you!

I tried to bump promesa a while ago in this project (which is used by several projects: nbb, scittle, joyride, etc. so people can use promesa with SCI in those projects).

The first error I get is this one:

WARNING: Use of undeclared Var promesa.exec/UnsupportedOperationException at line 634 cljs-test-runner-out/promesa/exec.cljc
WARNING: Use of undeclared Var promesa.exec/UnsupportedOperationException at line 634 cljs-test-runner-out/promesa/exec.cljc
#error {
 :cause Unable to resolve var: error in this context at line 175 /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs
 :data {:file /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 175, :column 19, :tag :cljs/analysis-error}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message failed compiling file:/home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs
   :data {:file #object[java.io.File 0x53348b2d /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs], :clojure.error/phase :compilation}
   :at [cljs.compiler$compile_file$fn__4807 invoke compiler.cljc 1768]}
  {:type clojure.lang.ExceptionInfo
   :message nil
   :data #:clojure.error{:source /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 175, :column 19, :phase :compilation}
   :at [cljs.analyzer$analyze_seq_STAR__wrap invokeStatic analyzer.cljc 4073]}
  {:type clojure.lang.ExceptionInfo
   :message Unable to resolve var: error in this context at line 175 /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs
   :data {:file /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 175, :column 19, :tag :cljs/analysis-error}
   :at [cljs.analyzer$error invokeStatic analyzer.cljc 780]}]
 :trace

See https://github.com/babashka/sci.configs/actions/runs/5860187517/job/15887736487

Please let me know what I should do to preserve compatibility with both promesa 9 and 11.

This is only one of several steps, but let's tackle it one bit a time. Thanks for your help.

[niwinz]

I will look on it ASAP, tomorrow probably

El lun., 14 ago. 2023 22:14, Michiel Borkent @.***> escribió:

@niwinz https://github.com/niwinz OK, let's elaborate one bit at a time, thank you!

I tried to bump promesa a while ago in this project (which is used by several projects: nbb, scittle, etc. so people can use promesa with SCI in those projects).

The first error I get is this one:

WARNING: Use of undeclared Var promesa.exec/UnsupportedOperationException at line 634 cljs-test-runner-out/promesa/exec.cljc WARNING: Use of undeclared Var promesa.exec/UnsupportedOperationException at line 634 cljs-test-runner-out/promesa/exec.cljc

error {

:cause Unable to resolve var: error in this context at line 175 /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs :data {:file /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 175, :column 19, :tag :cljs/analysis-error} :via [{:type clojure.lang.ExceptionInfo :message failed compiling file:/home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs :data {:file #object[java.io.File 0x53348b2d /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs], :clojure.error/phase :compilation} :at [cljs.compiler$compile_file$fn4807 invoke compiler.cljc 1768]} {:type clojure.lang.ExceptionInfo :message nil :data #:clojure.error{:source /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 175, :column 19, :phase :compilation} :at [cljs.analyzer$analyze_seq_STARwrap invokeStatic analyzer.cljc 4073]} {:type clojure.lang.ExceptionInfo :message Unable to resolve var: error in this context at line 175 /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs :data {:file /home/runner/work/sci.configs/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 175, :column 19, :tag :cljs/analysis-error} :at [cljs.analyzer$error invokeStatic analyzer.cljc 780]}] :trace

See https://github.com/babashka/sci.configs/actions/runs/5860187517/job/15887736487

Please let me know what I should do to preverse compatibility with both promesa 9 and 11.

This is only one of several steps, but let's tackle it one bit a time. Thanks for your help.

— Reply to this email directly, view it on GitHub https://github.com/babashka/sci.configs/issues/21#issuecomment-1677995263, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGN7KPR556AYBGJ6UMVQV3XVKBLFANCNFSM6AAAAAA3PZZMP4 . You are receiving this because you were mentioned.Message ID: @.***>

[niwinz]

Restored the missing funcion, can you try to bump to 11.0.674? (tellme how can I run the action myself)

[borkdude]

I bumped it in the bump-promesa branch. You can run tests in the branch with bb test. I'm getting:

Error building classpath. Could not find artifact funcool:promesa:jar:11.0.674 in central (https://repo1.maven.org/maven2/)

but I'll try via a git sha.

[niwinz]

There has been an unfortunate inconvenience, the artifact should already be in clojars :D

[borkdude]

Via git sha it works:

funcool/promesa {:git/url "https://github.com/funcool/promesa"
                         :git/sha "765e51fefdd23d775e6233edd838ffc1c116419d"}

The next one:

$ bb test
Cloning: https://github.com/funcool/promesa
Checking out: https://github.com/funcool/promesa at 765e51fefdd23d775e6233edd838ffc1c116419d
WARNING: Use of undeclared Var promesa.exec/UnsupportedOperationException at line 634 /Users/borkdude/.gitlibs/libs/funcool/promesa/765e51fefdd23d775e6233edd838ffc1c116419d/src/promesa/exec.cljc
WARNING: Use of undeclared Var promesa.exec/UnsupportedOperationException at line 634 /Users/borkdude/.gitlibs/libs/funcool/promesa/765e51fefdd23d775e6233edd838ffc1c116419d/src/promesa/exec.cljc
#error {
 :cause Unable to resolve var: -bind in this context at line 201 /Users/borkdude/dev/sci.configs/src/sci/configs/funcool/promesa.cljs
 :data {:file /Users/borkdude/dev/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 201, :column 11, :tag :cljs/analysis-error}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message failed compiling file:/Users/borkdude/dev/sci.configs/src/sci/configs/funcool/promesa.cljs
   :data {:file #object[java.io.File 0x568bbf44 /Users/borkdude/dev/sci.configs/src/sci/configs/funcool/promesa.cljs], :clojure.error/phase :compilation}
   :at [cljs.compiler$compile_file$fn__4807 invoke compiler.cljc 1768]}
  {:type clojure.lang.ExceptionInfo
   :message nil
   :data #:clojure.error{:source /Users/borkdude/dev/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 201, :column 11, :phase :compilation}
   :at [cljs.analyzer$analyze_seq_STAR__wrap invokeStatic analyzer.cljc 4073]}
  {:type clojure.lang.ExceptionInfo
   :message Unable to resolve var: -bind in this context at line 201 /Users/borkdude/dev/sci.configs/src/sci/configs/funcool/promesa.cljs
   :data {:file /Users/borkdude/dev/sci.configs/src/sci/configs/funcool/promesa.cljs, :line 201, :column 11, :tag :cljs/analysis-error}
   :at [cljs.analyzer$error invokeStatic analyzer.cljc 780]}]
 :trace

Feel free to iterate in a branch and just mention the new commit sha, then you don't have to push to clojars for each change.

[niwinz]

ACK, I will try to run tests locally also. Thanks

[borkdude]

I'm starting to see why things broke. The macros are based on the old code. I thought I needed to expose -bind but this is only because macros like do! use it, so I just have to update those. Let me try that.

[niwinz]

I think the protocols should not be exposed. I have done this changes locally and worked OK

[niwinz]

https://github.com/babashka/sci.configs/commit/95f4ea3c44700af725173ed7d081af9937c6dafe is this ok for you?

[borkdude]

@niwinz Thanks for the help. I already fixed most things here now:

https://github.com/babashka/sci.configs/pull/23 (check the diff in the PR)

One minor but possibly breaking thing:

I used to be able to use raw promise interop on return values of promesa but now it's a wrapped object. It seems .then and .catch work fine, but .finally doesn't so I had to use p/finally here:

https://github.com/babashka/sci.configs/commit/0dc6c2cfa53e58627e348cbab4411f6f15eb4b83#diff-8d90d8df7aa2be02dea9780367c87c2f578a4e3de1263f62ad5a0e921517970cR26

[borkdude]

I think the protocols should not be exposed. I have done this changes locally and worked OK

Alright, that's fine with me but maybe then remove them from the public API docs and possible prefix the namespace with impl as well?

https://funcool.github.io/promesa/latest/

[borkdude]

@niwinz I merged my branch. If you want to do those macro changes as a separate PR , please go ahead and remove the protocol / impl namespace stuff. I can also just copy what you did to save you time. Either way is fine with me. Thanks a lot!

So it seems the only real breakage was the removal of error which is fine with me as well btw.

Sorry for the drama :)

[olymk2]

Thought I would give this a try, Am using these deps

        funcool/promesa {:mvn/version "11.0.674"}
        org.babashka/sci.configs {:git/url "https://github.com/babashka/sci.configs"
                                  :sha "172830879ad578c0de68f7438da6a15c6f66f957"}

I am seeing this error from sci, the only .promesa call I have currently is promesa/let.

message: "Could not resolve symbol: promesa.protocols/-mcat

[borkdude]

That's strange since this is the code I'm testing in the tests:

(ns example
  (:require
    [promesa.core :as p]))

(p/do
1 2 3
(p/let [x (p/resolved (inc 2))]
  (inc x)))

[borkdude]

I'm going to port @niwinz's changes to do and let.

[borkdude]

@olymk2 please try again with 766a3c2748705985221c6c1625cd071b15f21ab6

[niwinz]

Between meeting and meeting, I haven't had time to do any MR and you've already made all the changes hehe

[niwinz]

One minor but possibly breaking thing:

I used to be able to use raw promise interop on return values of promesa but now it's a wrapped object. It seems .then and .catch work fine, but .finally doesn't so I had to use p/finally here:

I'm looking on it right now FYI

[olymk2]

@borkdude that's changed my error if nothing else :)

Could not resolve symbol: promesa.core/bind

Just looking at your PR I noticed you changed mcat to bind, its almost like I don't have access to the namespace, I will remove any caches I can find see if it makes any difference

[borkdude]

wtf.. it's clearly here: https://github.com/babashka/sci.configs/blob/766a3c2748705985221c6c1625cd071b15f21ab6/src/sci/configs/funcool/promesa.cljs#L172

@olymk2 is it possible to make a repro project so I can take a look? it sounds like something fishy might be going in, possible CLJS compilation caches or so?

[borkdude]

@niwinz I get another issue when trying to advance compile with promesa:

SEVERE: /Users/borkdude/dev/sci.configs/cljs-test-runner-out/promesa/impl/promise.js:53:17: ERROR - [JSC_LANGUAGE_FEATURE] This language feature is only supported for ECMASCRIPT_2020 mode or better: Nullish coalescing.
  53|         resolve: resolve ?? defaultResolveMapHandler,

You can repro this with bb test:advanced on main.

[olymk2]

wtf.. it's clearly here:

https://github.com/babashka/sci.configs/blob/766a3c2748705985221c6c1625cd071b15f21ab6/src/sci/configs/funcool/promesa.cljs#L172

@olymk2 is it possible to make a repro project so I can take a look? it sounds like something fishy might be going in, possible CLJS compilation caches or so?

Sure I sent you a PM with a link on slack as the repo is not on github but hosted on a private gitea server.

[borkdude]

@awb99 Your missing functions are added in commit 07866bbe0ebd82f884b1d1fb08a9acb34842b73f

[borkdude]

Diagnosed @olymk2's problem:

The problem in your setup is that you're adding the promesa namespace under promesa and not promesa.core

;; 'reagent sci-reagent/reagent-namespace ;; 'promesa sci-promesa/promesa-namespace

you should add the namespace under the original namespace name, else macro-expansions won't work like they're supposed to if you want to create aliases, you can do so via :aliases {'promesa 'promesa.core}

[borkdude]

@niwinz Another breakage in nbb:

This function used to return true and now returns falls with promesa return values:

(defn await? [p]
  (and (instance? js/Promise p)
       (.-__nbb_await_promise_result ^js p)))

So the instance check for js/Promise fails. Could the wrapped promesa promise perhaps extend js/Promise (even though it doesn't use any of the native promise functions) so instance checks still work?

[borkdude]

Note that I can't call p/promise? here since this code must be able to function without promesa.

[niwinz]

@niwinz Another breakage in nbb:

This function used to return true and now returns falls with promesa return values:

(defn await? [p]
  (and (instance? js/Promise p)
       (.-__nbb_await_promise_result ^js p)))

So the instance check for js/Promise fails. Could the wrapped promesa promise perhaps extend js/Promise (even though it doesn't use any of the native promise functions) so instance checks still work?

In this case, you can just check for "then" function exists on the object, it does not need to be instance of js/Promise

[borkdude]

What if a user defines an object with a then property which is not a promise? Seems a little brittle?

[borkdude]

I went with this solution, but am slightly worried about that edge case.

[niwinz]

This is how it works in js, if you await any object with a then property it will call it expecting this to work

[borkdude]

Amazed again by how brittle JS is.

You can "await" anything non-promise:

var z = {then: 1, else: 2} await z { then: 1, else: 2 }

But let's change the value of that randomly named "then" property to a function

var z = {then: (x) => x, else: 2} await z ;; JS freezes

[borkdude]

@niwinz Anyway thanks a lot for your help!

I think there's only one issue left then:

https://github.com/babashka/sci.configs/issues/21#issuecomment-1680363104

[niwinz]

@niwinz I get another issue when trying to advance compile with promesa:

SEVERE: /Users/borkdude/dev/sci.configs/cljs-test-runner-out/promesa/impl/promise.js:53:17: ERROR - [JSC_LANGUAGE_FEATURE] This language feature is only supported for ECMASCRIPT_2020 mode or better: Nullish coalescing.
  53|         resolve: resolve ?? defaultResolveMapHandler,

You can repro this with bb test:advanced on main.

The main issue here is that I'm using modern operator on the js file ?? https://caniuse.com/mdn-javascript_operators_nullish_coalescing https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing

Is there any problem with it?

[niwinz]

I mean, I can take some time to change it to more "compatible" approach, but looks like decent/secure to use browsers already support this operator

[borkdude]

Browser aren't the only environments that run JS. But I checked Node and this is supported since Node 14, which is already EOL, so all good I think. It seems we're done here. Many thanks again.