build: cacao patch v1.1.1 clockskew #2226
Conversation
zachferland
commented
Jun 14, 2022
•
zachferland
commented
- cacao patch v1.1.1 clockskew
- dids v3.2.0 with cacao phaseout
- abort controller lib change
zachferland
force-pushed
the
build/cacao-patch-1.1.1-clockskew
branch
from
bf9226a
to
5d62b03
Compare
zachferland
force-pushed
the
build/cacao-patch-1.1.1-clockskew
branch
from
5d62b03
to
7aec638
Compare
|
hmm not sure about ci build issue yet, do not see this locally |
| x: bigint, | ||
| y : bigint |
There was a problem hiding this comment.
yeah one of the lint rules, wanting lowercase types for primitives (bigint being prim in es2020), one of the lint dependency updates must have caused it to start throwing errors/warnings
There was a problem hiding this comment.
Why is your linter different from the rest of the teams?
There was a problem hiding this comment.
I don't understand why this change is needed based on the changes you did in the code.
|
Ran into this issue, when updating dependencies - mysticatea/abort-controller#36 In last commit, changed abort controller library, not sure other options if we didnt want to do this. It is a different abort controller library than one used in ipfs (original here). |
|
@zachferland can you explain why this change made the abort controller issue show up? I don't see you using abort controller in your code anywhere, is it used in one of our dependencies? If it's one of the dependencies we control, should we fix it there? |
|
@stbrody yeah its only due to upgrading dependencies (ref issue linked above, also seen in earlier CI fail), likely due to node types or typescript dependencies upgrading (not sure exactly unfortunately). But the current abort-controller library wont align on types now until they change it. even if we tried to only up dependencies needed here in package lock, would likely run into same issue soon we use abort-controller in common |
|
basically node type and abort-controller pollyfill/lib dont match types now |
|
FYI I'd like to do a release tomorrow with fixes for gitcoin and ideally this would be included in it if we can get to consensus on the changes in time |
|
I am not sure what happened there, but apparently it was something weird, considering changes in abort-controller package and package-lock file. A bit too suspicious for my taste. Maybe simpler #2237 could work so that @zachferland does not have to redo the work in this PR? |
|
I do not know, maybe different version of npm contributed to the confusion? |
|
yeah same feeling, glad to defer to another pr that works and minimizes changes but would expect to see it come up later was using npm 8.11.0, node v16.15.1, seen in CI as well |
|
Can we close it now @zachferland ? |
|
Closing, as another PR overrides this. |
