Rest-deployment.yaml fails due to OOM Killed

Please update the resources available on you official webpage as the resources that are being set for the resources is hugely to low, not only for rest-deployment but also for other components. When working with such resources they like to fail on large workloads and this for sure gives people the bad feeling that Mayastor is incomplete.
Referring to: https://mayastor.gitbook.io/introduction/quickstart/deploy-mayastor
In site header: REST
Link: https://raw.githubusercontent.com/openebs/mayastor-control-plane/v1.0.4/deploy/rest-deployment.yaml
There you've got:
      containers:
        - name: rest
          resources:
            limits:
              cpu: 100m
              memory: 64Mi
            requests:
              cpu: 50m
              memory: 32Mi

In our case the minimum required setup for the REST Deployment (Pod) to get out of OOMKill loop was:
      containers:
        - name: rest
          resources:
            limits:
              cpu: 1500m
              memory: 2048Mi
            requests:
              cpu: 500m
              memory: 256Mi

And the above was only to get out of the Mayastor deployment stage.
Lab-Environment, Kubernetes is a bare metal kubespray deployment:
kubernetes: "v1.25.3"
nodes:
- name: node_group_1
  count: 4
  linux_distro: "Ubuntu 22.04"
  hardware:
    cpu_type: "Intel Xeon ICX"
    cpu_count: 1
    ram: "512GB"
    drives:
    - device: "nvme0"
      size: "349.32GB"
    - device "sda"
      size: "256GB"
- name: node_group_2
  count: 4
  linux_distro: "Ubuntu 22.04"
  hardware:
    cpu_type: "Intel Xeon ICX"
    cpu_count: 2
    ram: "1024GB"
    drives:
    - device: "nvme0"
      size: "1.46TB"
    - device: "nvme1"
      size: "1.46TB"
    - device: "nvme2"
      size: "1.46TB"
    - device: "nvme3"
      size: "1.46TB"
    - device: "sda"
      size: "447GB"

Update: Everything works like a charm after limits upgrade. We have tripled all other pods resources requests.
NAMESPACE   NAME                NODE        STATUS   CAPACITY        USED   AVAILABLE
mayastor    r2s13-r32-nvme1n1   r2s13-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s13-r32-nvme2n1   r2s13-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s13-r32-nvme3n1   r2s13-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s14-r32-nvme1n1   r2s14-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s14-r32-nvme2n1   r2s14-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s14-r32-nvme3n1   r2s14-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s15-r32-nvme1n1   r2s15-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s15-r32-nvme2n1   r2s15-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s15-r32-nvme3n1   r2s15-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s16-r32-nvme1n1   r2s16-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s16-r32-nvme2n1   r2s16-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r2s16-r32-nvme3n1   r2s16-r32   Online<PHONE_NUMBER>592   0<PHONE_NUMBER>592
mayastor    r3s02-r32-sda       r3s02-r32   Online<PHONE_NUMBER>48    0<PHONE_NUMBER>48
mayastor    r3s03-r32-sda       r3s03-r32   Online<PHONE_NUMBER>48    0<PHONE_NUMBER>48
mayastor    r3s04-r32-sda       r3s04-r32   Online<PHONE_NUMBER>48    0<PHONE_NUMBER>48

@Mionsz this should be improved on 1.0.5 by limiting the  maximum number of rest workers. We were creating a worker on each logical core. I cherry-picked a fix from v2.
That said, on v1 we're still using NATS, so it's possible that memory usage may still grow. If you get a chance to try 1.0.5 (will be released soon) with the default limits when it's out, please update this ticket with your findings.

Closing this now since there's no updates, please do reopen if otherwise. Thanks!

@tiagolobocastro Yes, sorry for no new updates. This was probably fixed as I have managed to deploy 8 node cluster of Mayastor without any issues some time ago. Thanks.


Home Assistant Can't Connect

I've been running Home Assistant on a pi with zwavejs2mqtt on a separate raspberry pi in a docker container. It's configured to use UI only. It's been working for over a year. I have Watchtower configured to auto-update the zwavejs2mqtt container. Ever since the update last night, (updated to <IP_ADDRESS>fcfdd4) home assistant reports that it cannot connect. The container is running and I can connect to the UI, but it seems all my nodes have disappeared.
The container logs show:
2023-08-08 19:15:13.798 INFO APP: Application path:/usr/src/app
2023-08-08 19:15:13.849 WARN STORE: scenes.json not found
Logging to file:
        /usr/src/app/store/logs/zwavejs_2023-08-08.log
2023-08-08 19:24:20.067 INFO Z-WAVE-SERVER: Server closed
2023-08-08 19:24:40.510 INFO APP: Version: <IP_ADDRESS>fcfdd4
2023-08-08 19:24:40.533 INFO APP: Application path:/usr/src/app
2023-08-08 19:24:40.656 WARN STORE: scenes.json not found
Logging to file:
        /usr/src/app/store/logs/zwavejs_2023-08-08.log

I tried to check the file indicated in the log (using sudo nano) but that directory doesn't exist. I'm a bit clueless at this point.

Did some more digging and was able to read the zwavejs_2023-08-08.log. It looks like after the update, the container is no longer communicating with the nodes. For example at the bottom of the log is says that Node 9 is trying to communicate but is unknown.  Why doesn't the 'latest' pull work? Is 8.22.3 a beta?

I had the same issue last night (about 12 hours ago) - updated my stack through Portainer and ended up with 8.22.1 which was broken. Repeating the update now pulled 8.22.3 and everything is working again.

Feel free to reopen if the issue persists with latest 8.22.3


[Crash] [Mod Compat] Ender IO Conduit sound crash.

Mod Version:
3230-1.10
Forge Version:
<IP_ADDRESS>5
Link to crash log (if applicable):
http://pastebin.com/QKpschak
Description:
Crash upon walking on Ender IO conduits. Might be a good idea to have some kind of error suppressor for blocks without a sound associated, because this seems to be the issue.

Is this a public pack or something you put together?

It was a modified version of the Direwolf20 pack. This was specifically caused by me walking ontop the EI/O conduit. Thanks for fixing this btw.

Pushed <IP_ADDRESS> BETA to CurseForge with the fix.


Interface 'IMidiPermissionDescriptor' incorrectly extends interface 'PermissionDescriptor'

What is the current behavior?

Steps to reproduce it and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have extra dependencies other than react-use. Paste the link to your JSFiddle or CodeSandbox example below:
What is the expected behavior?
A little about versions:

OS:
Browser (vendor and version):
React:
react-use:
Did this worked in the previous package version?

We're also seeing this in react-use 17.3.1 alongside typescript 4.4.2. Linting passes because CI still uses typescript 4.1.1. The error comes from:
https://github.com/streamich/react-use/blob/2cd91281afc5f252d9387dd4761e6f326b574f2c/src/usePermission.ts#L11-L19
That points us to #2063. @gelove what would be the right solution? It’s strange that lib.dom.d.ts sets
type PermissionName = "gamepad" | "geolocation" | "notifications" | "persistent-storage" | "push" | "screen-wake-lock";

Not sure how to solve the typings apart from:
interface IMidiPermissionDescriptor extends Omit<PermissionDescriptor, "name"> {
    name: 'midi';
    sysex?: boolean;
}
interface IDevicePermissionDescriptor extends Omit<PermissionDescriptor, "name"> {
    name: 'camera' | 'microphone' | 'speaker';
    deviceId?: string;
}

Would that be semantically correct?

I've tested this out with TS 4.4 and this works too: https://gist.github.com/scvnathan/e9ff3a784fda80b408f514ab6f3acc68
I'm guessing these permission types are experimental so the TS team removed them

@scvnathan sounds reasonable! Would you be willing to submit a PR? 👍
We can place type PermissionDesc = after interface Experimental...Descriptor definitions and possibly keep the original name (IPermissionDescriptor) to minimise the diff. TBH I prefer unabbreviated names without I/T prefix like ExtendedPermissionDescriptor, but we can leave this for later 🙂

Sure, I can submit a PR 👍

Hi! @react-hookz/web, the new library by one of react-use's former maintainers (background here and here) has resolved this outstanding type issue. It might be worth taking a look.
For those interested, there's an official migration guide for migrating from react-use to @react-hookz/web.

Is this library offically unmaintained now? This bug is blocking our build pipelines - we can put in workarounds for now but it's not ideal.

@leepowelldev Unfortunately this library is not "officially unmaintained", as no one has been able to establish contact with @streamich (the project owner). As far as I can tell, every couple months he appears, accepts a PR or two, then disappears again. Personally, I tried contacting him multiple times to see if I could help with the project backlog. No luck. So I'd say this project is "for all intents and purposes, unmaintained".

@JoeDuncko Sure. Kind of annoying as I'd happily move to @react-hookz/web, but the reality is many of the hooks we use in this library are not yet implemented.

@lee1994522 if you compile a list of react-use hooks you use that are not yet in @react-hookz/web and send it my way? It'd help us figure out what hooks we should be focusing on next. Thanks!

@JoeDuncko - thanks for this, just gone through and seems the only one we're missing that would allow us to migrate is useWindowSize, if I have time over the next day or two I can pull together a PR for this unless you already have one in the works?

@JoeDuncko - thanks for this, just gone through and seems the only one we're missing that would allow us to migrate is useWindowSize, if I have time over the next day or two I can pull together a PR for this unless you already have one in the works?

Awesome! I don't think we have it actively in the works, so a PR would be appreciated!

@kachkaev Hello, Is there a plan to merge the PR and release a new version?

@Rey-Wang I believe that https://github.com/streamich/react-use/pull/2101 should fix this issue. I am able to merge the PR, but only @streamich can publish a new version of react-use on npm. Looks like we’ll need to wait for his help here.

@leepowelldev Unfortunately this library is not "officially unmaintained", as no one has been able to establish contact with @streamich (the project owner). As far as I can tell, every couple months he appears, accepts a PR or two, then disappears again. Personally, I tried contacting him multiple times to see if I could help with the project backlog. No luck. So I'd say this project is "for all intents and purposes, unmaintained".

I just started using this lib and ran into this build problem (annoyingly I don't even use usePermission, but it still won't build because of it). Disappointing that such a great lib is being thrown away due to inactive maintenance. GitHub should introduce some sort "claim abandoned repo" feature (yes, I know you can fork but it's not really the same). Ah well, guess I'll have to switch to @react-hookz/web...

As a workaround to this you can change your imports to direct imports:
// @deprecated author doesn't maintain react-use actively.
// use @react-hookz/web instead
import useKey from 'react-use/lib/useKey';
import useDebounce from 'react-use/lib/useDebounce';
import useClickAway from 'react-use/lib/useClickAway';
import useTimeoutFn from 'react-use/lib/useTimeoutFn';
import useEffectOnce from 'react-use/lib/useEffectOnce';
import useWindowSize from 'react-use/lib/useWindowSize';
import usePreviousDistinct from 'react-use/lib/usePreviousDistinct';

export { useKey, useDebounce, useClickAway, useTimeoutFn, useEffectOnce, useWindowSize, usePreviousDistinct };

As long as you're not using usePermission you can stop the build issue. You can migrate to new library as they become available or start to write your own replacements.

As a workaround to this you can change your imports to direct imports:
// @deprecated author doesn't maintain react-use actively.
// use @react-hookz/web instead
import useKey from 'react-use/lib/useKey';
import useDebounce from 'react-use/lib/useDebounce';
import useClickAway from 'react-use/lib/useClickAway';
import useTimeoutFn from 'react-use/lib/useTimeoutFn';
import useEffectOnce from 'react-use/lib/useEffectOnce';
import useWindowSize from 'react-use/lib/useWindowSize';
import usePreviousDistinct from 'react-use/lib/usePreviousDistinct';

export { useKey, useDebounce, useClickAway, useTimeoutFn, useEffectOnce, useWindowSize, usePreviousDistinct };

As long as you're not using usePermission you can stop the build issue. You can migrate to new library as they become available or start to write your own replacements.

Thanks!


Macro: RNA Builder. Delete presets.

Background:
This task covers deleting RNA presets in RNA library.
Requirements:
User deletes RNA presets in the following way:
•	User hovers over chosen RNA preset in preset section in RNA library.

•	User clicks on Delete button.
•	Once delete button is clicked, the warning message appears: “Are you sure you want to delete RNA preset?”. (TBD with UX)
•	Two following options appear: Yes and No.
•	User confirms to delete RNA preset by clicking “Yes”.
•	RNA preset is removed from RNA library
User chooses not to delete presets from RNA library:
•	User confirms not to delete RNA preset by clicking “No”.
•	RNA preset stays in RNA library
Inability to delete and edit default RNA presets:
•	User can not delete/edit default RNA presets, buttons are disabled
Displaying a star for adding presets to favorites:

A star for adding RNA presets to Favorite library shall appear in the upper left corner inside preset framework.

** Figma link
https://www.figma.com/file/MZ28kJRelnXvBodXu54UKh/Macromolecules-(Polymer-Editor)-Playground?type=design&node-id=5149-9013&mode=design&t=1YYvaoEyNpvgCShy-0

context menu positioning

Tested

I just couldn't add RNA presets to Favorite library


Refactor KojiImportPlugin to reduce complexity...

...accrued due to if logic surrounding source
container builds.

Associated fixes for unit tests

i.e. implement
class KojiImportBase
|- class KojiImportPlugin
|- class KojiImportSourceContainerPlugin

CLOUDBLD-121

Signed-off-by: Ben Alkov<EMAIL_ADDRESS>Maintainers will complete the following section

[ ] Commit messages are descriptive enough
[ ] Code coverage from testing does not decrease and new code is covered
[ ] JSON/YAML configuration changes are updated in the relevant schema
[ ] Changes to metadata also update the documentation for the metadata
[ ] Pull request has a link to an osbs-docs PR for user documentation updates
[ ] New feature can be disabled from a configuration file

@reviewers
pylint's complaints are:

atomic-reactor-fedora-29-py2

atomic_reactor/plugins/exit_koji_import.py:380: [E1101(no-member), KojiImportBase.combine_metadata_fragments] Instance of 'KojiImportBase' has no 'get_build' member; maybe '_get_build'?)
atomic_reactor/plugins/exit_koji_import.py:470: [E1101(no-member), KojiImportBase.run] Instance of 'KojiImportBase' has no 'get_server_dir' member)

atomic-reactor-fedora-31-py3

atomic_reactor/plugins/exit_koji_import.py:380: [E1101(no-member), KojiImportBase.combine_metadata_fragments] Instance of 'KojiImportBase' has no 'get_build' member; maybe '_get_build'?)
atomic_reactor/plugins/exit_koji_import.py:470: [E1101(no-member), KojiImportBase.run] Instance of 'KojiImportBase' has no 'get_server_dir' member)

I'd like to waive them, given that

I wrote this based on my understanding of Python's classes
All unit tests pass
CPython doesn't seem to care nearly as much about this as pylint does :)

@ben-alkov IMO pylint is right here.
You are calling the get_build() and get_server_dir() methods in the parent class KojiImportBase, but they are only defined in the child classes. If they do need to be called in the base class, they should be defined there, perhaps as abstract if no default definition makes sense - e.g.:
def get_server_dir(self):
    # Must be implemented by subclasses
    raise NotImplementedError

@ben-alkov IMO pylint is right here.
You are calling the get_build() and get_server_dir() methods in the parent class KojiImportBase, but they are only defined in the child classes. If they do need to be called in the base class, they should be defined there, perhaps as abstract if no default definition makes sense - e.g.:
def get_server_dir(self):
    # Must be implemented by subclasses
    raise NotImplementedError

OHH! That's what I was trying to remember... :)
I don't believe that they are ever called from the base class, but I wanted to stub them - I forgot about NotImplementedError

The 5 new lines not covered by unit tests are all method stubs in the parent class which raise NotImplementedError.

LGTM

The refactoring looks great!
I'd like to to test this before merging, and for that, we will also need a patch to osbs-client updating the plugins configuration for source container builds.

Incoming!


Dockerfile.nipapd Fails to build

Trying to build this dockerfile fails. Below is the bottom of the build output where the error occurs.
#6 121.6 Collecting envtpl
#6 121.9   Downloading https://files.pythonhosted.org/packages/02/d6/c00aac9a35972a6c0fcf55afdf2f61c9316a6b688db01185c948d9d78b26/envtpl-0.6.0.tar.gz
#6 122.1 Requirement already satisfied (use --upgrade to upgrade): argparse>=1.0 in /usr/lib/python2.7 (from envtpl)
#6 122.1 Collecting Jinja2>=2.7 (from envtpl)
#6 122.2   Downloading https://files.pythonhosted.org/packages/89/e3/b36266381ae7a1310a653bb85f4f3658c462a69634fa9b2fef76252a50ed/Jinja2-3.1.1.tar.gz (267kB)
#6 122.4   Requested Jinja2>=2.7 from https://files.pythonhosted.org/packages/89/e3/b36266381ae7a1310a653bb85f4f3658c462a69634fa9b2fef76252a50ed/Jinja2-3.1.1.tar.gz#sha256=640bed4bb501cbd17194b3cace1dc2126f5b619cf068a726b98192a0fde74ae9 (from envtpl), but installing version None
#6 122.4 Collecting MarkupSafe>=2.0 (from Jinja2>=2.7->envtpl)
#6 122.6   Downloading https://files.pythonhosted.org/packages/1d/97/2288fe498044284f39ab8950703e88abbac2abbdf65524d576157af70556/MarkupSafe-2.1.1.tar.gz
#6 122.7     Complete output from command python setup.py egg_info:
#6 122.7     Traceback (most recent call last):
#6 122.7       File "<string>", line 1, in <module>
#6 122.7       File "/tmp/pip-build-BWyhJE/MarkupSafe/setup.py", line 26
#6 122.7         raise BuildFailed() from e
#6 122.7                                ^
#6 122.7     SyntaxError: invalid syntax
#6 122.7
#6 122.7     ----------------------------------------
#6 122.7 Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-BWyhJE/MarkupSafe/
#6 123.0 You are using pip version 8.1.1, however version 22.0.4 is available.
#6 123.0 You should consider upgrading via the 'pip install --upgrade pip' command.
------
executor failed running [/bin/sh -c apt-get update -qy && apt-get upgrade -qy  && apt-get install -qy devscripts     make     libpq-dev     libsqlite3-dev     postgresql-client     python     python-all     python-docutils     python-pip     python-dev  && pip --no-input install envtpl  && rm -rf /var/lib/apt/lists/*]: exit code: 1

Dockerfile.www also has a similar error

I'm running into this as well. raise .. from is a new feature since Python 3, and most (all?) of NIPAP is written in Python 2. I think it is caused by the pip --no-input install envtpl, which pulls the latest envtpl, which is based on Python 3.
It could be fixed by pinning the causing dependency to a suitable Python 2 predecessor, or (IMHO the better option but far longer option) wait for #1316

With my PR, I am able to build both containers


Compatibility issue with newer Chromium

I get a lot of:
[Violation] Added non-passive event listener to a scroll-blocking 'touchstart' event. Consider marking event handler as 'passive' to make the page more responsive. See https://www.chromestatus.com/feature/5745543795965952

in console when using your library in a toy project.
This is the line that triggers it:
this.listElement.addEventListener('touchstart', this.handleDragStartTouch);
Sorry, I have no clue about why this happens or whether that's not a problem (I see that you do add passive: true for touchmove event), I'm a backend guy mostly.

Hi :)
Are you able to repro that on the demo page: https://marekrozmus.github.io/react-swipeable-list/ ?
Which version of Chromium exactly - I tried the demo page on just downloaded canary version and nothing in console 🤷♂
Maybe it is some specific case of scrollable area in scrollable area - just guessing.

The version is Version 100.0.4896.127 (Official Build) (64-bit), I can repro it on the demo page actually:

you need to help me out here - I cannot find that version (got only newer) and not quite sure which Chromium channel are you using. If it possible please provide me a link :)

Well, it's an ArchLinux, pacman tells me this:
λ pacman -Q google-chrome
google-chrome 100.0.4896.127-1

I have tested with Chromium on Windows and Mac and I have no warnings.
I cannot test it with Linux. I am wondering why the linux version is so "old" - like "100" - I have at least "101".
If the lib will cause you some other problems then please let me know.

I Can reproduce this warning message on the Demo Page using Chrome 108.0.5359.99 on Windows 11.
Seems like react-swipeable has included a solution for this problem in their latest 7.0.0 release^1 to prevent these warnings, and thus the point subtraction from the PageSpeed audit score.


Green status icon shown even when the chain is behind

Describe the bug
Blocks behind on the main page is quite large on the dev instance (91), but has a green status next to it.
To Reproduce
Steps to reproduce the behavior:

Go to main dashboard page, and observe the status icon on the top when node is behind.

Expected behavior
When the node is behind, it should show a warn status, perhaps ember icon
Screenshots

Desktop (please complete the following information):

MacOS
Chrome
Version 115.0.5790.170

This issue was due to slot times varying from local testnet to mainnet/goerli. Due to the inconsistency we decided to use the sync_distance provided by the beacon sync api endpoint.


SAM-3161: 'Save and publish the assessment' button is disabled

Yeah looks like a regression I introduced from KNL-945 thx for finding and fixing!


Cleaned up python code in the dev branch

Changes

Cleaned up code in bot.py, general.py, veteran.py, and moderation.py

Created isAuthorized() to remove redundant code
Removed and rewritten code to be concise

Change so $botreports command reports to #dev-team channel

Hopefully this is helpful

There's a problem with the PR. It seems that none of the cogs get loaded successfully. Please fix that issue then when you are finished post a comment here and I can finish testing :+1:

After further testing, the separation of commands with roles does not work. Another issue is that some commands were restricted to only run in the #bot-commands channel however in this PR they are not. There is a big release set for the bot coming up soon and this still has bugs that need to be worked out. Closing PR please make another one when everything is fixed.


IdP url opened twice

Driver version
2.0.882
Redshift version
Not a Redshift problem
Client Operating System
MacOS Big Sur
Python version
3.7.7
Table schema
Not a Redshift problem
Problem description
Connecting to a Redshift cluster with credentials being provided by IdP (Okta in this case) results in Okta login_url page being opened twice if I have already signed in previously. To make it a bit more confusing, when redirected back to http://localhost:7890/redshift/ the page shows "This site can't be reached". Connection is established after that with no issues and I can execute the query.
I've also setup a connection in DBeaver using the same IdP and the same login_url. In this case the login_url page is opened only once and a proper message saying Thank you for using Amazon Redshift! You can now close this window. is shown.

Expected behaviour:
IdP login_url is opened once. After successful login and redirect to http://localhost:7890/redshift/, the page shows the same Thank you for using Amazon Redshift! You can now close this window.

Actual behaviour:
IdP login_url is opened twice. When redirected to http://localhost:7890/redshift/ it shows "This site can't be reached"

Error message/stack trace:
No errors other than "This site can't be reached" in the browser

Any other details that can be helpful:

Python Driver trace logs
None
Reproduction code
import redshift_connector

conn = redshift_connector.connect(
    iam=True,
    ssl=True,
    host="REDSHIFT_ENDPOINT",
    port=5439,
    database="DB_NAME",
    db_user="IDP_USERNAME",
    region="AWS_REGION",
    cluster_identifier="CLUSTER_NAME",
    login_url="IDP_LOGIN_URL",
    credentials_provider="BrowserSamlCredentialsProvider",
    user="",
    password=""
)

cursor: redshift_connector.Cursor = conn.cursor()
cursor.execute("select current_user")

result = cursor.fetchone()
print(result)

Hi @ivica-k ,
Thank you for reaching out. At this time, redshift_connector is not displaying the "Thank you for using Amazon Redshift! You can now close this window." message following authentication. The team plans to fix this issue as the current behavior is confusing and not inline with our Amazon Redshift JDBC driver.

Connecting to a Redshift cluster with credentials being provided by IdP (Okta in this case) results in Okta login_url page being opened twice if I have already signed in previously.

I will try and reproduce this behavior on my machine and get back to you with my findings. Thank you for including the screen capture. Aside from the additional un-expected re-direct, is the authentication process successful in this case?

HI @ivica-k ,
I was unable to reproduce the multi-redirect behavior you provided in the screen capture. To start off, I want to confirm that a new browser tab being opened for every execution of the included Python script is to be expected.
Here are the steps I followed, please let me know if I missed something.
My machine: Mac OD 10.15.7
My browser:  Google Chrome 91.0.4472.114
My Okta login URL: https://${MY_OKTA}.okta.com/app/${MY_SSO}

I cleared the cookies for Google Chrome
I navigated to https://${MY_OKTA}.okta.com/ and signed in using IDP_USERNAME and my password. This re-directed me to https://${MY_OKTA}.okta.com/UserHome. I left this browser tab open.
I invoked the script you provided above and authenticated with Okta

I saw the following behavior when invoking the script.

A new browser tab was opened, using IDP_LOGIN_URL
Quickly, this browser tab re-directed to http://localhost:7890/redshift/ and displayed the connection refused message. I was not prompted to authenticate with Okta, as the browser cookies were used.
Python console indicated the program executed without issue.

Going forward...

If you clear your browser's cookies and execute the Python script do you see this issue?
If you execute the script again, without clearing your browser's cookies do you see this issue?
Could there be some additional re-direct configured in the Okta IdP settings?

Looking at the BrowserSamlCredentialsProvider class, redshift_connector performs actions to:

open a web broswer to IDP_LOGIN_URL
run a server on localhost

just one time, then waits for the SAML response to be received by this localhost server -- see code here.

Hey @Brooke-white and thank you for being so prompt!
I followed your advice, which is:

Cleared all cookies
Logged into Okta.com and left https://bestseller.okta.com/app/UserHome open
Invoked the same script I provided in my previous message

Unfortunately this did not help - a new browser tab opened IDP_LOGIN_URL once, displayed the connection refused message, another tab opened the same IDP_LOGIN_URL, displayed the message again and then my script completed and gave the query result.
I was not prompted to authenticate twice - but the IDP_LOGIN_URL was opened twice. After debugging I could clearly see that the [def authenticate()](https://github.com/aws/amazon-redshift-python-driver/blob/master/redshift_connector/plugin/browser_saml_credentials_provider.py#L58) was invoked twice.
I've turned on debug logging for that specific plugin and this is what is shown:
2021-07-05 13:39:12,517 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - Listen_port=7890
2021-07-05 13:39:12,517 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - Login_url=https://${MY_OKTA}.okta.com/app/${APP_ID}/sso/saml
2021-07-05 13:39:12,517 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - Idp_response_timeout=120
2021-07-05 13:39:12,806 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - Listening for connection on port 7890
2021-07-05 13:39:12,809 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - SSO URI: https://${MY_OKTA}.okta.com/app/${APP_ID}/sso/saml
2021-07-05 13:39:14,303 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - Data received contained SAMLResponse: True
2021-07-05 13:39:17,256 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - Listening for connection on port 7890
2021-07-05 13:39:17,256 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - SSO URI: https://${MY_OKTA}.okta.com/app/${APP_ID}/sso/saml
2021-07-05 13:39:18,081 - redshift_connector.plugin.browser_saml_credentials_provider - DEBUG - Data received contained SAMLResponse: True

Authentication works and I can see the query result.
I've also tried using a different (freshly installed) browser and the same thing happens.

Hi @ivica-k ,
Thank you for providing the debug log. BrowserSamlCredentialsProvider utilizes a cache to store the temporary AWS credentials retrieved after authenticating with the identity provider. There are two cases where the SSO URI would be opened twice

The credentials retrieved from Okta expired before the authentication process could complete
The cache is misbehaving somehow -- this would cause an additional request for credentials to be made, which would open the Okta login URL again.

I've had the first case occur when debugging the Identity Provider plugin code, but I would not expect this case to typically occur as the authentication process should not take much time :)
As for a misbehaving cache, I'd be interested to learn more about the following...
If a breakpoint is set within SamlCredentialsProvider.get_credentials() on line 57 the expected behavior is that this line will be executed 2 times and both values of key will be the same. See here for how the cache key is generated.
For example:
key = '${USERNAME}${PASSWORD}None443900None'
Additionally, I'd suggest checking the contents of self.cache within this same method. Expect to see the above key value in theSamlCredentialProvider.cache. Exploring the entry for the above key, I can see an entry credentials which is a dictionary. Within that dictionary I see the following:
{
  'AccessKeyId': 'XXXX',
  'SecretAccessKey': 'XXXX',
  'SessionToken': 'XXXX',
  'Expiration': datetime.datetime(2021, 7, 7, 23, 18, 37, tzinfo=tzutc())
}

Are the keys in this credentials dictionary inline with those described above? Following the second execution of Line 57 what is the value of self.cache[key].is_expired()? I would expect this to be False.
There is an entry for Expiration in SamlCredentialProvider.cache as well, it's expected to be inline with the value provided in the credentials dictionary, if present.

Thank you for the great explanation, you obviously know this library intimately. In the end, I believe timezones and timezone-aware Expiration field is the problem. As you probably know, AWS clocks are all in UTC, leading to this:
now: 2021-07-09 16:37:23.890818, expiration: 2021-07-09 15:37:18+00:00

My timezone is Europe/Belgrade at the moment and it's 16:38 (4:38 PM). Credentials are "expired" as soon as they are created.
I would love to submit a PR for this but would need a bit of guidance. Do you have a preferred timezone library, should I only use what stdlib offers etc.

Hi @ivica-k ,
Thank you for taking the time to dive into this issue. I am happy we have been able to determine a root cause for this issue...this also explains why I was unable to reproduce as my timezone is PST :)
The team would be happy to have your contribution for this issue -- the edit you provided looks good!
Back to the "Connection Refused" screen in the browser following authentication, the team has merged a fix for this and it will be included in our next release, which is scheduled for July 19.

So, I was able to make it work in my timezone with this simple diff
     def is_expired(self: "CredentialsHolder") -> bool:
-        return datetime.datetime.now() > self.expiration.replace(tzinfo=None)
+        import time
+        utc_offset = time.localtime().tm_gmtoff // 60 // 60
+
+        return (datetime.datetime.now() - datetime.timedelta(hours=utc_time)) > self.expiration.replace(tzinfo=None)

What should happen with self.expiration in this case? Should it be changed based on the local timezone? Or should I maybe add self.expiration_local and change self.expiration to self.expiration_utc ?

Hey @ivica-k ,
Every CredentialsHolder object has an expiration. You can think of the CredentialsHolder class as wrapper around the temporary credentials retrieved when using IAM authentication.

What should happen with self.expiration in this case? Should it be changed based on the local timezone? Or should I maybe add self.expiration_local and change self.expiration to self.expiration_utc ?

Let's chat more in the PR, but I think we can keep expiration as is and do a conversion to the local timezone within is_expired(). We should probably add a logging.debug statement within this method which outputs the original UTC datetime and the localized one. This could be helpful to future debugging surrounding this area of the code base :)
Thanks again for diving into this issue, the team really appreciates your help!


Conversion not starting when googleStorageBucket is set

Everything works quite fine, but when I do want to convert a *.pkpass-File (it is a Season-Ticket including a Photo and a Logo) the error message is always the same:
Error: Could not load the default credentials. Browse to https://cloud.google.com/docs/authentication/getting-started for more information.

If the googleStorageBucket-Value is not set, the Pass gets generated and could be added to Google Wallet.
Any ideas on how I could make it work? googleServiceAccountJsonPath and googleIssuerId is set correctly.

Not entirely certain but it might help if you create a seperate script that saves a file to Google Cloud using the credentials/bucket you've configured - I imagine you'll get the same errors and if you can get that working, you'll solve the larger issue.

It's to do with the request to STORAGE.
I added a config item for the Google Project ID to the config.json
// Google Cloud project id   "googleProjectId": "",
And then amended app.js to account for the new way of authenticating
// Create a Cloud Storage client     const storage = new Storage({projectId: config.googleProjectId, keyFilename: config.googleServiceAccountJsonPath });
And that fixed the issue for me

I suspect this arises when separate cloud projects are used. I'll close for now.


NuGet package has a broken dependency

Hi.
Veldrid depends on a missing package System.Text.Primitives (>= 0.1.0-e170627-2).

Hey @orthoxerox , sorry about that. The package is actually available from this myget feed: https://dotnet.myget.org/gallery/dotnet-corefxlab.
It is a bit inconvenient, but that functionality should be moving into nuget.org at some point, so it won't always be on a third-party feed.


Enums with methods

Hello,
I was lacking the possibility of having methods and properties to enums, so here is a proposal that adds this functionality. Additionally, the documentation of documented members is now also present in the ".pyi" output.
Here is a summary of the few changes:

MemberInfo and MemberDef have a doc and a is_property attribute.
StubInfoBuilder::add_methods adds member properties for enums (but not for classes, the behaviour remains the same)
StubInfoBuilder::add_methods now looks for struct_id in module.enum_ in addition to module.class.

I would really like these changes to make it to a new release, please let me know if additional changes are required to comply with the current code base.

Hello @termoshtt,
Is there any progress on this ? Do you need any input from my side ?
I would really like to merge this feature such that my projects can depend on this repo instead of my current fork.
Here is an example of what the generated pyi looks like in one of my projects (for the Action enum class).
Best regards,
Yannick


Added stdio option description in the readme

As a following of this issue, the stdio documentation should be in the readme as newer people don't know about it.

@marcofugaro Are you having trouble with Node v5.0?
I just upgraded and stdio: 'ignore' is no longer suppressing output.

@colinrotherham Damn they might have changed something with the stdio option in Node 5, or it might be a problem on their end that need to be reported.
Maybe this plugin needs a proper option like logging: false.
I've tried Node 5 but I couldn't get my libsass to work, so I downgraded to the 4.2.2

@marcofugaro Seems to read like it's the same. I don't know enough about it though.
(Presuming you use Gulp, make sure you use gulp-sass v2.1.0 and it works fine with Node v5. You'll need to remove your node_modules dir and npm install again to resolve. It was fixed in node-sass v3.4.1)


KAFKA-13968: Fix 3 major bugs of KRaft snapshot generating

More detailed description of your change
There are 3 bugs when a broker generates a snapshot.

Broker should not generate snapshots until it starts publishing.
Before a broker starts publishing, BrokerMetadataListener._publisher=None,  so _publisher.foreach(publish) will do nothing, so featuresDelta.metadataVersionChange().isPresent is always true, so we will generating a snapshot on every commit since we believe metadata version has changed, here are the logs, note offset 1 is a LeaderChangeMessage so there is no snapshot:

[2022-06-08 13:07:43,010] INFO [BrokerMetadataSnapshotter id=0] Creating a new snapshot at offset 0... (kafka.server.metadata.BrokerMetadataSnapshotter:66)
[2022-06-08 13:07:43,222] INFO [BrokerMetadataSnapshotter id=0] Creating a new snapshot at offset 2... (kafka.server.metadata.BrokerMetadataSnapshotter:66)
[2022-06-08 13:07:43,727] INFO [BrokerMetadataSnapshotter id=0] Creating a new snapshot at offset 3... (kafka.server.metadata.BrokerMetadataSnapshotter:66)
[2022-06-08 13:07:44,228] INFO [BrokerMetadataSnapshotter id=0] Creating a new snapshot at offset 4... (kafka.server.metadata.BrokerMetadataSnapshotter:66)

We should compute metadataVersionChanged before _publisher.foreach(publish)
After _publisher.foreach(publish) the BrokerMetadataListener_delta is always Empty, so metadataVersionChanged is always false, this means we will never trigger snapshot generating even metadata version has changed.

We should try to generate a snapshot when starting publishing
When we started publishing, there may be a metadata version change, so we should try to generate a snapshot before first publishing.

Summary of testing strategy (including rationale)
A unit test.
Committer Checklist (excluded from commit message)

[ ] Verify design and implementation
[ ] Verify test coverage and CI build status
[ ] Verify documentation (including upgrade notes)

Hello, @mumrah the first bug is related to KIP-778, @jsancio you may be familiar with the second and third bugs, my solution for these problems are as follows:

Do not generate snapshots before publishing and try to generate a snapshot when starting publishing.
invoke maybeStartSnapshot() before publish()

@hachikuji The cause ofRaftClusterSnapshotTest.testSnapshotsGenerated has been found and the other is still under investigation.

Reran the Jenkins build. It looks like the build is timing out. Maybe this is because we have a lot more tests after migrating some tests to run against KRaft and ZK.

KAFKA-14073 is created for log improvement.


Clarification regarding predicate pushdown

Does this library take into account the stats field present in the transaction log of a particular table version to filter the parquet files that are supposed to be read while instantiating a DeltaTable?
For example, I have a table with total 100 records across 2 table versions and each version has 5 parquet files associated with it covering 50 records of that version (assuming even 10 records per parquet file). The transaction log for both versions contains a stats field with information like minValues, maxValues, nullCount etc.
I have already verified that if I try to read first version (0), the DeltaTable object will read only 1 files and for second version (1) it will read both the files. This means that the number of parquet files are affected by the version already.
Just needed a clarification whether the stats fields are also used anywhere for selection of files to be read?

Data skipping using stats in Delta Sharing is not supported yet.

@zsxwing can you make any statement where this is on the roadmap? Having data skipping in spark could be super useful for performance... I can also open another issue if thats preferred.


ray_intersect not working on Firefox & Ubuntu 22.04

Hi, first of all thanks for a great library!
Please forgive me, that I am not seasoned with either graphics programming, wasm or windowing systems in Linux, so this issue might be due to user error :)
I however ran into an when determining the 3d coordinate for the cursor projected on top of a desired object. I am using ray_intersect for calculating the point of collision. However on my system on Firefox 119.0.1, the intersection point is always the position of the camera. This seems to originate from the shader or somewhere else further down the chain, as adding logging to the functions shows that the rendered pixel is indeed exactly zero.
On Chromium the same setup works as expected with the intersection point being correctly on the surface of the geometry.
I am running Ubuntu 22.04 with X11 windowing system. My GPU is RTX 2080, and glxinfo | grep "OpenGL version" returns OpenGL version string: 4.6.0 NVIDIA 525.147.05

Thanks for reporting! You're right, picking is not working in firefox, works fine in chrome and safari 🤷 Fortunately firefox shows a relevant warning, so shouldn't be too hard to fix. I'll do it as soon as I have a bit of time 🙂

Thanks for confirming! I did some digging around and managed to get it to work on Firefox as well.
Anyway, as the error message WebGL warning: readPixels: Format and type RED/FLOAT incompatible with this R32F attachment. This framebuffer requires either RGBA/FLOAT or getParameter(IMPLEMENTATION_COLOR_READ_FORMAT/_TYPE) RGBA/FLOAT. suggests, Firefox would like the return type of readPixels to be RGBA/FLOAT.
So I modified core.rs, line 209 in format_from_data_type case 1 to return crate::context::RGBA as well.
After this I received another warning about too small buffer. This was easily solved by modifying render_target.rs, and multiplying data_size in read_color_partially with 4.
Hope this helps with the fixing :)

Rendering with NormalMaterial might be affected by a similar issue in both Chromium and Firefox, at least according to my testing.
I am trying to read the normals by the following method:
pub fn camera_normals(
    context: &Context,
    position: Vec3,
    direction: Vec3,
    geometries: impl IntoIterator<Item = impl Geometry>,
) -> Vec3 {
    use three_d::core::*;
    let width = 15;
    let height = 15;
    let viewport = Viewport::new_at_origo(width, height);
    let up = if direction.dot(vec3(1.0, 0.0, 0.0)).abs() > 0.99 {
        direction.cross(vec3(0.0, 1.0, 0.0))
    } else {
        direction.cross(vec3(1.0, 0.0, 0.0))
    };
    let camera = Camera::new_orthographic(
        viewport,
        position,
        position + direction * 100.,
        up,
        0.01,
        0.0,
        100.,
    );
    let mut texture = Texture2D::new_empty::<Vec4>(
        context,
        viewport.width,
        viewport.height,
        Interpolation::Nearest,
        Interpolation::Nearest,
        None,
        Wrapping::ClampToEdge,
        Wrapping::ClampToEdge,
    );
    let normal_material = NormalMaterial {
        ..NormalMaterial::default()
    };

    let normals: Vec<f32> = texture.as_color_target(None)
    .clear(ClearState::color_and_depth(1.0, 1.0, 1.0, 1.0, 1.0))
    .write(|| {
        for geometry in geometries {
            render_with_material(context, &camera, &geometry, &normal_material, &[]);
        }
    }).read();

    let mut ret = Vec3 {x: 0., y: 0., z: 0.};

    for i in 0..width*height {
        ret.x += normals[(i * 4) as usize];
        ret.y += normals[(i * 4 + 1) as usize];
        ret.z += normals[(i * 4 + 2) as usize];
    }

    ret /= (width*height) as f32;
    ret * 2. - vec3(1. , 1., 1.)
}

, but the Firefox debug console shows a similar result as with the depth: WebGL warning: readPixels: Format and type RED/FLOAT incompatible with this RGBA32F attachment. This framebuffer requires either RGBA/FLOAT or getParameter(IMPLEMENTATION_COLOR_READ_FORMAT/_TYPE) RGBA/FLOAT..
Again if I apply the aforementioned changes, the errors go away.
Furthermore the depth and normals seems to be unstable to some degree, will attempt to dig further into these.

I finally, after three months, got a bit of time to fix this. Ray intersect is working after 5d52c48. The problem is that it's not possible to read a float from each pixel on Firefox, but it is possible to read 4 floats from each pixel. So the fix in you're code is simply to replace let normals: Vec<f32> =.. with let normals: Vec<[f32; 4]> =... In the same commit, I've updated the documentation and added a panic on web if you try to read less than 4 values from a render target.

Oh and by the way, it's not a good idea to read the normals to the CPU if you want your application to perform. It's generally a bad idea to read from the GPU to the CPU unless you really need it 🙂 It's probably more efficient (and easier) to compute the normals on the CPU if you need them there.

Thank you for taking the time to fix this, and your instructions! :)
My goal for the normal reading is to step a few units back from the intersection point between pointer and object, so I am only reading the normals for a single pixel. Thus haven't at least noticed any performance issues. Good to know anyway if I run into issues on less performant hardware.

No problem 🙂 Ah cool. Yeah, reading the normal of a single pixel to CPU should perform just fine.


allow recursive generic type aliases

interface A<a> {
    brand: 'a';
    nested: a;
}

interface B<a> {
    brand: 'b';
    nested: a;
}

type C<a> = A<a> | B<a>;

type D = C<D> | string; // <-- i wish

While we had a long discussion about this on #3496, we closed that because the original issue was unblocked. We can certainly continue discussion a bit more. Is there a specific scenario you're interested in modeling here?

As I recall, this happens because type aliases do not actually create a type. Furthermore, the type argument position causes a circularity because the only way to look up the type reference in the instantiation cache is to know the id of each type argument. It is pretty much a consequence of the caching mechanism for generics.

Say you have a cyclic object graph of nodes defined like this:
interface Node { id: number; children: Node[]; }
When such graph gets serialized all cyclic references have to be encoded
with something different than Node:
interface Reference { nodeId: number; }
So ultimately the serializable object graph should be defined like this:
interface NodePlain { id: number; childern: (Node | Reference)[]; }
But practically  during deserializing we want to replace all Reference
object with resolved Node objects, going back to the original definition:
interface Node { id: number; children: Node[]; }
So instead of maintaining 2 interfaces (one for serializing, one for
deserializing) I wish I could parametrize the Node interface with a type of
node:
interface Draft { id: number; children: Child [] }
Then we have:
type Node = Draft ;
type NodePlain = Draft <NodePlain | Reference>
function serialize (value: Node): NodePlain {}
function deserialize (value: NodePlain): Node {}
On Dec 23, 2015 7:25 PM, "Jason Freeman"<EMAIL_ADDRESS>wrote:

As I recall, this happens because type aliases do not actually create a
type. Furthermore, the type argument position causes a circularity because
the only way to look up the type reference in the instantiation cache is to
know the id of each type argument. It is pretty much a consequence of the
caching mechanism for generics.
—
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/6230#issuecomment-167013105
.

More basic example
Json = string | number | boolean | Json [] | { [name: string]: Json }
On Dec 23, 2015 8:39 PM, "Aleksey Bykov"<EMAIL_ADDRESS>wrote:

Say you have a cyclic object graph of nodes defined like this:
interface Node { id: number; children: Node[]; }
When such graph gets serialized all cyclic references have to be encoded
with something different than Node:
interface Reference { nodeId: number; }
So ultimately the serializable object graph should be defined like this:
interface NodePlain { id: number; childern: (Node | Reference)[]; }
But practically  during deserializing we want to replace all Reference
object with resolved Node objects, going back to the original definition:
interface Node { id: number; children: Node[]; }
So instead of maintaining 2 interfaces (one for serializing, one for
deserializing) I wish I could parametrize the Node interface with a type of
node:
interface Draft { id: number; children: Child [] }
Then we have:
type Node = Draft ;
type NodePlain = Draft <NodePlain | Reference>
function serialize (value: Node): NodePlain {}
function deserialize (value: NodePlain): Node {}
On Dec 23, 2015 7:25 PM, "Jason Freeman"<EMAIL_ADDRESS>wrote:

As I recall, this happens because type aliases do not actually create a
type. Furthermore, the type argument position causes a circularity because
the only way to look up the type reference in the instantiation cache is to
know the id of each type argument. It is pretty much a consequence of the
caching mechanism for generics.
—
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/6230#issuecomment-167013105
.

In your serialization example, I think you could get by with:
interface Node extends Draft<Node> { }
interface NodePlain extends Draft<NodePlain | Reference> { }

As long as there is only one sort of Draft, yes I can do it. In a non
trivial case I cannot:
type Node = OneDraft | AnotherDraft

Why can't you do
interface Node extends OneDraft<Node>, AnotherDraft <Node> { }

Because I am looking for a sum type (either or) not a product type (this
and that).
On Dec 24, 2015 5:57 PM, "Jason Freeman"<EMAIL_ADDRESS>wrote:

Why can't you do
interface Node extends OneDraft, AnotherDraft  { }
—
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/6230#issuecomment-167164813
.

Oh sorry, you're right. I misread it.

Well, the reason it happens is what I said before. Hopefully that can provide a clue about how to fix it.

As discussed in #7489, this issue is related to the special case of partial application in #5453, albeit indirectly.

I have a slightly different case where the current work-around is not satisfying. Consider two types following the work-around pattern:
type StringOrStringTree = (string | StringTree);
interface StringTree extends Array<StringOrStringTree> {}

type NumberOrNumberTree = (number | NumberTree);
interface NumberTree extends Array<NumberOrNumberTree>{}

Now what I want to write is a flatten function that flattens a tree into non-nested array such as (without types) would be:
function flatten(items) {
  let result = [];
  function flattenNode(node) {
    if (Array.isArray(node)) {
      node.forEach(flattenNode);
    }
    else {
      result.push(node);
    }
  }
  flattenNode(items);
  return result;
}

It is clear this would work correctly for both types but is there is no good typing for this function that allows both types and infers T without someway to express a recursive union type. Maybe something like:
function flatten<T>(tree: T | this[]: T[] {
  ....
}

would work where T | this[] would match a recursive union such as:
type StringOrStringTree = string | StringOrStringTree[];

where the this refers to the union type. Maybe self would be better here to avoid collision with other uses of this in a type position.
This example requires a way to express the type (which the work-around gives me) and a way of expressing the type relation implied by the type for type inferencing (which the work-around doesn't give me).
Here is some brain-storming around potential syntaxes:
  function flatten<T>(tree: T | this[]): T[];
  function flatten<T>(tree: T | self[]): T[];
  function flatten<T>(tree: T | union[]): T[];
  function flatten<T, S = T | S[]>(tree: S): T[];

The last one above is the most general solution, allows expressing the type relation directly, but might make the implementation too difficult as it would allow any type relation expressible in using a type alias. The advantage of using a special symbol is it allows the type to be written as a stand-alone expression without need for a meta-symbol in situation where a meta-symbol would be awkward (e.g. foo(a: string | this[])) .

@chuckjaz

T | this[]

:-1: for anonymous recursive references. I don't see much of a use case for that, and it feels like a solution in search of a problem.
It also reminds me too much of arguments.callee conceptually.

@isiahmeadows I don't have a problem with it being named only I couldn't come up with a good way to do it. The last example was the best I could come up with but it is, as you can see, fairly ugly and only works in the context of a type variables, not in a single expression.
Suggestions?

So far, the closest to feasible idea to come up is caching type aliases like how interfaces are already. Everything else so far that has been suggested along those lines have been declined AFAIK.
// Now
interface JSONObject {
  [key: string]: JSONValue;
}

interface JSONArray extends Array<JSONValue> {}

type JSONValue =
  string |
  number |
  boolean |
  JSONArray |
  JSONObject;

// If type aliases are cached like interface types
type JSONValue =
  string |
  number |
  boolean |
  JSONValue[] |
  {[key: string]: JSONValue};

I think the discrepancy of how type aliases and interfaces are cached is what's causing all these problems in the first place. Two code paths for similar concepts make it easy for this to happen.

I agree but, my point was that this is incomplete as it doesn't provide a way to declare a function that is generic over recursive union types. For this I need to be able to express a recursive type relation as well as the type itself, hence my examples.

Does my proposal not cover type Nested<T> = T | Nested<T>[], the type you need?

Sorry! I totally missed that. I should read more carefully.

Bump, this could be a super helpful extension to the type system and allow for all sorts of cool things languages like Haskell already allow for, like the Haskell list definition:
type List<T> = [T, List<T>] | void

Also, if this works (which it does):
type List<T> = { a: [T, List<T>] } | void

and this doesn't:
type List<T> = [T, List<T>] | void

Then this should probably be considered a bug and not a new feature.

@calebmer

Also, if this works (which it does): [code omitted] and this doesn't: [code omitted] Then this issue should probably be considered a bug and not a new feature.

I feel it's a bug your first one doesn't trigger the cycle detector. In no other kind of situation without an interface in the way have I found recursively referenced type aliases not fail to check.

But I do feel that caching type aliases similarly to how interfaces are already cached (or else, interface T<U extends T<U>> wouldn't work) would really help.

As for my idea, here's what should check, assuming Bar and Baz are interfaces:

type Foo = Bar - that's okay now.
type Foo = Bar | Baz<Foo> - Recursive case not in top level.
type Foo = Bar | Foo[] - Recursive case not in top level (Foo[] and Array<Foo> are the same).
type Foo = Bar<Foo> - Recursive case not in top level.
type Foo = Bar<Baz | Array<Foo>> - Recursive case not in top level.
type Foo = Array<Bar | Foo> - Recursive case not in top level.
type Foo = {x: Foo} - Recursive case not in top level.

Here's what shouldn't check, following the same assumptions:

type Foo = Foo - Self-explanatory
type Foo = Bar | Foo - Recursive case on top level.
type Foo = Bar & Foo - Recursive case on top level.

Generics don't affect this at all, and for purposes of detecting cycles, the generic parameters are generally irrelevant. The only exception is that generic bounds limits cannot recursively reference the type at the top level, like in type Foo<T extends string | Foo<T>>. The only base case for that situation I know of is with F-bounded interface type like interface Bar<T extends Bar<T>>, and that's an exceptionally rare case to need it. (Well, technically, a string type works here as well...)
As for when multiple type aliases get involved, it should work by simply reducing the type aliases until they no longer contain any other type aliases at the top level. If any cycles come up, they should be reported.

@isiahmeadows I agree with the characterization of top-level versus non-top-level. That's a great way to put it.

@JsonFreeman Thanks! :smile:
I will point out that, as an odd edge case, it will allow type Foo = Foo[], but you can already use interface Foo extends Array<Foo> {} to a similar effect, even though it's a bit of an edge case itself.

I think it is good to allow type Foo = Foo[]. It is not at the top level

I was implying it's okay. It's just odd at the conceptual level (for the average programmer). Where would be the best way to seek more input on a proposal? There doesn't seem to be any clear place(s) from what I've found.

would like to bring this up again, the following is not possible to express without recursive type aliases
type Expression = Binary<Expression> | Unary<Expression> | Conditional<Expression>;

please re-consider

@aleksey-bykov True, and that could come up easily in basic AST interpreters, where the type union is better than extending an abstract class:
export interface String { type: "string"; value: string }
export interface Variable { type: "variable"; name: string }

export type Expression =
  Constant<string> |
  Constant<number> |
  Variable |
  Add<

@aleksey-bykov
First, there's nothing to reconsider at all, because nothing's been rejected (i.e. the "needs proposal" part). Just none of us here have enough time and/or knowledge to actually implement it.
Second, in your particular example, generics are generally rarely necessary in practice. That kind of problem comes up easily in basic recursive AST interpreters, where the type union is more type-safe and easier than extending an abstract class. Here's an example derived from this SO answer.
// Types
export interface Const { type: "const"; value: number }
export interface Var { type: "var"; name: string }
export interface Add { type: "add"; left: Node; right: Node }
export interface Sub { type: "sub"; left: Node; right: Node }
export interface Mul { type: "mul"; left: Node; right: Node }
export interface Div { type: "div"; left: Node; right: Node }
export interface Assign { type: "assign"; name: Var; value: Node }
export interface Loop { type: "loop"; cond: Node; body: Node }
export interface Seq { type: "seq"; body: Node[] }

export type Node =
  Const | Var |
  Add | Sub | Mul | Div |
  Assign | Loop | Seq

export type State = {[key: string]: number}

// Constructors
export const c = (value: number) => ({type: "const", value}) as Const
export const v = (name: string) => ({type: "var", name}) as Var
export const add = (left: Expr, right: Expr) => ({type: "add", left, right}) as Add
export const sub = (left: Expr, right: Expr) => ({type: "sub", left, right}) as Sub
export const mul = (left: Expr, right: Expr) => ({type: "mul", left, right}) as Mul
export const div = (left: Expr, right: Expr) => ({type: "div", left, right}) as Div
export const set = (name: string, value: Expr) => ({type: "assign", name: v(name), value}) as Assign
export const loop = (cond: Expr, value: Stmt) => ({type: "loop", cond, stmt}) as Loop
export const seq = (stmts: Stmt[]) => ({type: "seq", name, value}) as Seq

export function run(node: Node, state: State): Val {
  switch (node.type) {
  case "const": return node.value
  case "var":
    if (!{}.hasOwnProperty.call(state, node.name)) {
      throw new ReferenceError(`'${node.name}' is not set!`)
    }
    return state[node.name]
  case "add": return run(node.left, state) + run(node.right, state)
  case "sub": return run(node.left, state) - run(node.right, state)
  case "mul": return run(node.left, state) * run(node.right, state)
  case "div": return run(node.left, state) / run(node.right, state)
  case "assign": return state[node.name] = run(node.value, state)
  case "loop": {
    let result = 0
    while (run(node.cond, state)) result = run(node.body, state)
    return result
  }
  case "seq": {
    let result = 0
    for (const stmt of node.body) result = run(stmt, state)
    return result
  }
  }
}

// Example usage
const fib = seq([
  set("x", c(0)),
  set("y", c(1)),
  loop(v("n"), seq([
    set("t", add(v("x"), v("y"))),
    set("x", v("y")),
    set("y", v("t")),
    set("n", sub(v("n"), c(1))),
  ])),
  v("x"),
])

run(fib, {n: 25}) // 75025

The above in OCaml would look something like this:
type State = (string, int) Hashtbl.t
type Node =
  | C of int
  | V of string
  | Add of Node * Node
  | Sub of Node * Node
  | Mul of Node * Node
  | Div of Node * Node
  | Set of string * Node
  | While of Node * Node
  | Seq of Node list
exception Unbound of string

let rec eval node state =
  match node with
    | C value -> value
    | V value ->
      try Hashtbl.find state value
      with Not_found -> raise (Unbound value)
    | Add (left, right) -> eval left state + eval right state
    | Sub (left, right) -> eval left state - eval right state
    | Mul (left, right) -> eval left state * eval right state
    | Div (left, right) -> eval left state / eval right state
    | Set (string, value) ->
      let result = eval value state in
      begin
        Hashtbl.replace state string result
        result
      end
    | While (cond, body) ->
      let result = ref 0 in
      begin
        while run cond state <> 0 do prev := run body state
        !result
      end
    | Seq body -> body |> List.fold_left (fun _ stmt -> run stmt state) 0

(* Example usage *)
let () =
  let fib = Seq [
    Set "x" (C 0);
    Set "y" (C 1);
    While (V "n") (Seq [
      Set "t" (Add (V "x") (V "y"));
      Set "x" (V "y");
      Set "y" (V "t");
      Set "n" (Sub (V "n") (C 1))
    ]);
    V "x"
  ] in
  let state : State = Hashtbl.create 10 in
  begin
    Hashtbl.add state "n" 25
    run fib state (* 75025 *)
  end

annoyingly enough this works:
interface Expression {
    myself: Binary<Expression> | Unary<Expression> | Conditional<Expression>;
}

@aleksey-bykov I earlier suggested caching aliases and interfaces the same way, which would fix the problem pretty simply. I don't have much experience with the compiler or type checker, or I would probably be willing to take a stab at it.

@isiahmeadows I actually like the caching aliases idea. It’s very annoying in declaration files that some of my larger union types are duplicated wherever it appears instead of being the cached union type.

It will involve introducing an actual type object in the compiler for type aliases. That is the main change as far as i know.

This side benefit is that the fly-over hints and error message would be much better as currently they show the expanded union instead of the alias. I am tending to use union types more an more and even mildly complicated alias types render the fly-over hints useless as well as make the type errors very difficult to read.

Yes, that is an understated benefit that would probably be quite useful.

@chuckjaz I agree that it is rather annoying. Even something at least like ``type Typeof = 'boolean' | 'string' | ...would be nice (and better than just a'boolean' | 'string' | ...` IMO).

this is the signature of webpack conditions:
interface ConditionSpec {
    /** The Condition must match. The convention is the provide a RegExp or array of RegExps here, but it's not enforced. */
    test?: Condition;
    /** The Condition must match. The convention is the provide a string or array of strings here, but it's not enforced. */
    include?: Condition;
    /** The Condition must NOT match. The convention is the provide a string or array of strings here, but it's not enforced. */
    exclude?: Condition;
    /** All Conditions must match. */
    and?: Condition[];
    /** Any Condition must match. */
    or?: Condition[];
    /** The Condition must NOT match. */
    not?: Condition;
}
type Condition = string | RegExp | ((absPath: string) => boolean) | ConditionSpec | Condition[];

is there any trick to make this possible or do i have to resort to any?

Try,
interface ConditionSpec {
    /** The Condition must match. The convention is the provide a RegExp or array of RegExps here, but it's not enforced. */
    test?: Condition;
    /** The Condition must match. The convention is the provide a string or array of strings here, but it's not enforced. */
    include?: Condition;
    /** The Condition must NOT match. The convention is the provide a string or array of strings here, but it's not enforced. */
    exclude?: Condition;
    /** All Conditions must match. */
    and?: Condition[];
    /** Any Condition must match. */
    or?: Condition[];
    /** The Condition must NOT match. */
    not?: Condition;
}
interface ConditionArray extends Array<Condition> { }
type Condition = string | RegExp | ((absPath: string) => boolean) | ConditionSpec | ConditionArray;

then it just say that test recursively references itself.
i created a hack that allows shallow type safety (and just allows any one level deeper`, but to faithfully represent this (really not that complex type), i fear i’d need this feature…

@flying-sheep I am using this style in one of my projects and it works (though my example is not as complex as yours) and my example works on the play ground and in VS Code using TypeScript 2.0. What am I missing?

oh? must be a VS code bug then:
there i get [ts] 'test' is referenced directly or indirectly in its own type annotation.

Can you make this into a branch I can pull?

https://github.com/flying-sheep/DefinitelyTyped/blob/webpack-2-correct/webpack/index.d.ts#L224-L239

The problem seems to be some weird interaction between the RegExp type and the optional test member  of the  ConditionSpec interface. If you make test required (remove the ?), or remove RegExp from Condition, or change test to not be the same of any member of RegExp, String or Function the error goes away.
@mhegazy Is this intentional. The error seems weird to me.

@chuckjaz sadly that’s not an an option, as this interface is correct as it is.
hmm, what i could do is
interface BaseConditionSpec {
    test?: Condition;
    include?: Condition;
    exclude?: Condition;
    and?: ConditionArray;
    or?: ConditionArray;
    not?: Condition;
}
interface TestConditionSpec { test: Condition }
interface IncludeConditionSpec { include: Condition }
[...]
type ConditionSpec = TestConditionSpec | IncludeConditionSpec | [...];

which would also be correct (an empty object would probably be legal, but useless)

This seems to work:
    /** The Condition must match. The convention is the provide a string or array of strings here, but it's not enforced. */
    include?: Condition;
    /** The Condition must NOT match. The convention is the provide a string or array of strings here, but it's not enforced. */
    exclude?: Condition;
    /** All Conditions must match. */
    and?: Condition[];
    /** Any Condition must match. */
    or?: Condition[];
    /** The Condition must NOT match. */
    not?: Condition;
}

interface TestConditionSpec {
    test: Condition;
}
interface ConditionArray extends Array<Condition> { }
type Condition = string | RegExp | ((absPath: string) => boolean) | TestConditionSpec | ConditionSpec | ConditionArray;

This implies that including test implies that you cannot also include or. That seems desirable because I am not sure what supplying and and or would mean, for example.

well, but test and include together do make sense.
i tried this, but it’s worse

How about:
    interface BaseConditionSpec {
        /** The Condition must match. The convention is the provide a string or array of strings here, but it's not enforced. */
        include?: Condition;
        /** The Condition must NOT match. The convention is the provide a string or array of strings here, but it's not enforced. */
        exclude?: Condition;
    }

    interface TestConditionSpec extends BaseConditionSpec {
        /** The Condition must match. The convention is the provide a RegExp or array of RegExps here, but it's not enforced. */
        test: Condition;
    }

    interface AndConditionSpec extends BaseConditionSpec {
        /** All Conditions must match. */
        and: ConditionArray;
    }

    interface OrConditionSpec extends BaseConditionSpec {
        /** Any Condition must match. */
        or: ConditionArray;
    }

    interface NotConditionSpec extends BaseCondtionSpec {
        /** The Condition must NOT match. */
        not: Condition;
    }

    interface ConditionArray extends Array<Condition> {}
    type Condition = string | RegExp | ((absPath: string) => boolean) | TestConditionSpec | OrConditionSpec | AndConditionSpec | NotConditionSpec| ConditionArray;

thanks, this did it!

Another use case I ran into, just in case it helps when evaluating potential solutions or raising the priority of this issue a bit:
I have a function that generates html from a tuple, where the first entry is the tag name, the second is the attributes (which can be omitted), and the third is entry is the contents (which can be another tag).
// the below generates <div id="adiv"><span>content!</span></div>
html(["div", {id: "adiv"}, ["span", "content!"]])

The signature for this function would be something like:
html(content: TagBlock)

where a TagBlock refers to a TagContents type, which indirectly refers to a TagBlock:
type TagBlock = [TagName, TagAttributes, TagContents] | [TagName, TagContents];

type TagName = 'div' | 'span' | ... ... | 'input';
type TagAttributes = any;
type TagContents = string | number | TagBlock;

Because interfaces require named properties, like object types, I don't see any way to describe this API but with type aliases (though correct me if I'm wrong).

I just ran in to this while trying to add types to a pretty simple recursive structure:
type Tree = string | string[];

I worked around it like this:
type GenericTree<T> = T | T[];
type Tree = GenericTree<string>;

what makes you think think your workaround works? it is literally the same
as the original type expression
On Sep 1, 2017 5:04 PM, "Brandon Bloom"<EMAIL_ADDRESS>wrote:

I just ran in to this while trying to add types to a pretty simple
recursive structure:
type Tree = string | string[];
I worked around it like this:
type GenericTree = T | T[];type Tree = GenericTree;
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/TypeScript/issues/6230#issuecomment-326683355,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA5Pzaog8jnSyh6TvROCTC6Fbk6k0GPOks5seHFhgaJpZM4G62Xy
.

Sorry, posted that comment by accident - wasn't done editing it.

type Flat<T> = T extends (infer P)[] ? Flat<P> : T;

error

Hi, this works (with --strict true):
type A = string | B;
interface B<T extends A = symbol> {
  a: T extends symbol ? A : T;
}

So...

@coolreader18 so what?
this works too: interface List<A> { value: A; rest: List<A> | null; }

Oh never mind whoops

Wait no because this:
type A = string | B;
interface B<T extends A = A> {
  a: T;
}

doesn't work.

so what is your point? linking a type to itself via interface isn't very useful of impressive, recursive interface's were here from version 0.8 of typescript, here we are talking about type's which are different in this regard, try using type instead of interface in your example and you will see what i am talking about

type Flat<T> = T extends (infer P)[] ? Flat<P> : T;

error
This works like a charm:

type InferDiscard<V> = V extends { _: infer T } ? T : V;
type Flat<T> = InferDiscard<T extends (infer P)[] ? { _: Flat<P> } : T>;
type Flatted = Flat<["a", ["b", ["c", "d"]]]>;
// Results in "a" | "b" | "c" | "d"

Thanks @calebmer for the hint. 😄

@devlato found a way around this that worked in TS < 3.2:
For the example in the description:
interface A<a> {
    brand: 'a';
    nested: a;
}

interface B<a> {
    brand: 'b';
    nested: a;
}

type C<a> = A<a> | B<a>;

type D<T extends D<T> = D<T>> = C<T> | string;

Unfortunately this no longer works in TS 3.3 - see #29803

Is there any way to currently express a type like the one described by @aleksey-bykov above?
type RawJSON =
  | null
  | string
  | number
  | { [key: string]: RawJSON }
  | RawJSON[];

While this is a bit contrived, my real-world use case is in representing GraphQL type definitions. This limitation makes it impossible to correctly type out lists. Currently I define an arbitrary, fixed number of type for each "depth", with the final one resolving unsafely to any. In practice I don't see arrays nested very deeply, so this is fine, but I dislike that a user will silently lose type safety if they do.

As a followup, I have a few questions that might help move this along:

Is there any theoretical objection to this? If this was correctly implemented in a PR, would it be considered?
Is there any practical limitation on implementing this? For example, are the internal data structures in TS unable to represent this?
Is there anybody actively working on this (either at MS or in the community)?

type RawJSON =
  | null
  | string
  | number
  | { [key: string]: RawJSON }
  | RawJSONArray;

interface RawJSONArray extends Array<RawJSON> {}

is the canonical example workaround.

Assuming we had the ability to abstract over type constructors, one approach that would make this easier from an implementation point of view would be to disallow arbitrary recursion, and instead have a special Fix type constructor baked in that acts as the equivalent of:
// :: type Fix : (* -> *) -> *
type Fix<F> = F<Fix<F>>

Then you can express any recursive type as a non-recursive type with a type parameter that gets applied away with the special Fix type, making it easy for the type checker to distinguish recursive from non-recursive types:
type RawJSONF<X> = | null
                   | string
                   | number
                   | { [key: string]: X }
                   | X[]

type RawJSON = Fix<RawJSONF>

@masaeedu Slight problem: you need some form of quantification plus some form of lambda-like abstraction for type-level operators. Typing that isn't exactly trivial, since you basically need a recursive type to define a type. (And by that point, it'd be easier to introduce dependent types.)
I would like higher-order generic type support, but a fixpoint combinator really isn't on that list due to pragmatic concerns around type theory.

@isiahmeadows I don't quite follow what you mean by "you basically need a recursive type to define a type". The proposal above doesn't require quantification, which is orthogonal in the lambda cube to type constructors, the only feature it does actually require. Given the ability to define non-recursive type constructors in user code, and a baked in fixpoint combinator for type constructors, you can express any recursive type constructor.

You're correct that the above doesn't require quantification, but that doesn't hold for the general case of the Y combinator, which supports arbitrary anonymous recursion, powerful enough it enables untyped lambda calculus to compute anything any Turing machine can.
But it's worth noting that you don't need unbounded recursion to support that type - a variant of this, generalized to limit recursion to just direct recursion. The Turing-completeness of TS's type system could then be safely broken by applying a similar check with indexed types. And although they do appear open to that in theory, they concluded that for the time being, "we're not quite ready for this sort of thing". (They weren't fond of the fact it seemed to complicate consumer code, and you'd have to fix that by normalizing conditional types to nest the conditions' results as far as possible, something TS doesn't currently do.)

but that doesn't hold for the general case of the Y combinator

Well no, not even the Y combinator at the type level requires quantification. Quantification or its absence is an orthogonal feature of a type system to the availability of and restrictions on type constructors.

powerful enough it enables untyped lambda calculus to compute anything any Turing machine can

It doesn't seem like Turing completeness is relevant here, given that the type system is already Turing complete. But while we're talking about irrelevant things: we don't need to talk about some other combinator to discuss this; introduction of Fix suffices to break strong normalization of a simply typed lambda calculus at the type level and make type checking undecidable.
Whether you allow:
type LList<X> = { case: "nil" } | { case: "cons", x: X, xs: LList<X> }

or:
type LList<X, R> = { case: "nil" } | { case: "cons", x: X, xs: R }
type List<X> = Fix<LList<X>>

does not have any bearing on the properties of the type system, it's purely an implementation and user experience concern. There are workarounds that let us formulate things in a way where the Fix combinator is more restrictive, but without even a basic kind system it's difficult to reason about out whether they prevent the introduction of Turing completeness. And besides, as I mentioned earlier, the type system is Turing complete anyway!

sorry for crashing the party, but i am not sure where it is all going, i only wish i could do the following:
type Expression = BinaryExpression<Expression> | UnaryExpression<Expression> | TrenaryExpression<Expresion> | Expression

am i asking for too much?

@aleksey-bykov I don't think so. Before we started talking about Turing completeness and all kinds of other irrelevant tangents, I was suggesting that it would be easier to instead implement:
type ExpressionF<X> = BinaryExpression<X> | UnaryExpression<X> | ...
type Expression = Fix<ExpressionF>

if perhaps less convenient to use.

looks like i found a workaround (sorry it's all messy but it works!)
function make<T>() {
    type BinExp = { left: Exp; right: Exp };
    type UnExp = { sole: Exp; };
    type Exp = BinExp | UnExp | T;
    return { Exp: null! as Exp, BinExp: null! as BinExp, UnExp: null! as UnExp, };
}
const R = make<string | number>();
type Exp = typeof R.Exp;
type BinExp = typeof R.BinExp;
type UnExp = typeof R.UnExp;
const un: UnExp = { sole: { left: 'a', right: 1 } }; // <---- YAY!

i've been pointed at a surprisingly simple answer, i am confused how i didn't see it before
type Exp<T> = BinExp<T> | UnExp<T> | T;
type UnExp<T> = { sole: Exp<T>; };
type BinExp<T> = { left: Exp<T>; right: Exp<T> };

I have a similar example:
type Num = {
    type: "number",
    value: number,
};

type Add<T> = {
    type: "add",
    args: T[],
};

type Mul<T> = {
    type: "mul",
    args: T[],
};

type ExprF<T> = Num | Add<T> | Mul<T>;

The reason why I don't do something like:
type Add<T> = {
    type: "add",
    args: ExprF<T>[],
};

Is that I want to be able to use these types for recursion schemes.  In particular I'd like to define cata a function which folds Expr into a ExprF<number> and returns the number (evaluate) or Expr into a ExprF<string> and returns the string (print) where Expr is defined by type Expr = ExprF<Expr>.
If we expand the definition of Expr once we get:
type Expr = Num | Add<Expr> | Mul<Expr>;

which doesn't work, but if we expand the definition again we get:
type Expr = {
    type: "number",
    value: number,
} | {
    type: "add",
    args: Expr[],
} | {
    type: "mul",
    args: Expr[],
};

which does work.  I wonder if the type checker could be modified to auto expand aliases when it encounters as recursive generic type aliases.

@kenhowardpdx have you looked at this: https://github.com/microsoft/TypeScript/issues/6230#issuecomment-480917723

@zpdDG4gta8XKpMCd the problem is that that formulation won't work with recursion schemes.  Given a function:
const exprCata = <A>(transform: ExprF<A> => A, expr: ExprF<Expr>): A => {
    return transform(fmap(x => exprCata(transform, x), expr));
}

where fmap is defined as:
const fmap = <A, B>(fn: A => B, expr: ExprF<A>): ExprF<B> => {
    switch (expr.type) {
        case "number":
            return expr;
                case "add";
        case "add":
            return {
                type: "add",
                args: expr.args.map(fn),
            };
        case "mul":
            return {
                type: "mul",
                args: expr.args.map(fn),
            };
        default:
            return (expr: empty);
    }
}

by defining ExprF as type ExprF<T> = Num | Add<T> | Mul<T>; we're able to control what type of data Add and Mul nodes contain.  Initially they contain Expr which is recursive but as exprCata runs, each Add/Mul node in the recursive structure is mapped to a non-recursive and then that non-recursive node is folded into into a single value.
Here's the rest of the code:
const evalTransform = (expr: ExprF<number>): number => {
    switch (expr.type) {
        case "number":
            return parseFloat(expr.value);
        case "add":
            return sum(expr.args);
        case "mul":
            return prod(expr.args);
        default:
            return (expr: empty);
    }
};

const ast = {
   type: "mul",
   args: [
       { type: "add": args: [{ type: "number", value: "2" }, { type: "number", value: "3" }]},
       { type: "number", value: "4" },
   ],
};

const result: number = exprCata(evalTransform, ast); // 20
// intermediary steps: (* (+ "2" "3") "4") => (* (+ 2 3) "4") => (* 5 4) => 20

Notice how evalTransform is not recursive.  The recursive mapping of the ast has been extracted into fmap.  exprCata allows us to reuse fmap and can be used to define other folds on the ast, e.g. pretty printing it.
The problem with:
type Exp<T> = BinExp<T> | UnExp<T> | T;
type UnExp<T> = { sole: Exp<T>; };
type BinExp<T> = { left: Exp<T>; right: Exp<T> };

is that it's always recursive which breaks recursion schemes.
The code I posted above works in Flow which got me thinking, why is it possible to declare recursive data types in Flow without running into a stack overflow?  I think the proposal I made to auto-expand type aliases could provide a way to do this in TypeScript.  I started prototyping the idea this weekend, but I'm not familiar with TypeScript internals so It's slow going.  I did make some progress though.  I was able to expand a the type aliases.  I still need to figure out how to create a copy of the expanded type and then substitute T for Expr in the copy.

I came up with a different solution to make recursion schemes work in TypeScript.  Instead of making the recursive type generic I define a helper type that replaces instances of a type (and arrays of the type) with a different type.  This is used to define a generic type ExprF<T> from a non-generic type Expr.
type ReplaceType<T, Search, Replace> = {
    [P in keyof T]: T[P] extends Search
        ? Replace
        : T[P] extends Search[]
            ? Replace[]
            : T[P];
};

type Expr = {
    kind: "add",
    args: Expr[],
} | {
    kind: "mul",
    args: Expr[],
} | {
    kind: "number",
    value: string,
};

type ExprF<T> = ReplaceType<Expr, Expr, T>;

Using these new types, here's the "evaluate" example from my previous post:
const fmap = <A, B>(
    fn: (a: A) => B,
    expr: ExprF<A>,
): ExprF<B> => {
    switch (expr.kind) {
        case "number":
            return expr;
        case "add":
            return {
                kind: "add",
                args: expr.args.map(fn),
            };
        case "mul":
            return {
                kind: "mul",
                args: expr.args.map(fn),
            };
        default:
            throw new UnreachableCaseError(expr);
    }
}

const exprCata = <A>(
    fmap: (fn: (a: Expr) => A, expr: Expr) => ExprF<A>,
    transform: (exprA: ExprF<A>) => A,
    expr: Expr,
): A => {
    return transform(fmap(x => exprCata(fmap, transform, x), expr));
}

const add = (a: number, b: number) => a + b;
const mul = (a: number, b: number) => a * b;
const zero = 0;
const one = 1;
const sum = (nums: number[]) => nums.reduce(add, zero);
const prod = (nums: number[]) => nums.reduce(mul, one);

const evaluateTransform = (expr: ExprF<number>): number => {
    switch (expr.kind) {
        case "number":
            return parseFloat(expr.value);
        case "add":
            return sum(expr.args);
        case "mul":
            return prod(expr.args);
        default:
            throw new UnreachableCaseError(expr);
    }
};

const evaluate = (ast: Expr) => exprCata(fmap, evaluateTransform, ast);

I think with a little more work a general version of exprCata is possible, but I'll leave that for another day.

Having to write type NestedMap<T> = Map<string, NestedMap<T>> as
type Node<T> = Map<string, T>;
interface NestedMap<T> extends Map<string, Node<T> | T> {}

(found solution in this thread)
Would be nice to have fixed :)

@snebjorn I wouldn't say that these represent the same type:

type NestedMap<T> = Map<string, NestedMap<T> | T>

actually means
type NestedMap<T> = Map<string, T>
                  | Map<string, Map<string, T>>
                  | Map<string, Map<string, Map<string, T>>>
                  | ...

with T at any depth, while

type Node<T> = Map<string, T>;
interface NestedMap<T> extends Map<string, Node<T> | T> {}

is just the same as
type NestedMap<T> = Map<string, T>
                  | Map<string, Map<string, T>>

@zepatrik oh crap, you're right.
Well then I guess this is the right place to ask for support for
type NestedMap<T> = Map<string, NestedMap<T> | T>

@snebjorn I presume you meant this as your solution?
interface Node<T> extends Map<string, Node<T> | T> {}

(This is a rare case where it's not quite so hacky to use the interface workaround.)

I'm facing an issue which seem related to this:
Given the following code for (that's not the real code it's just to illustrate with a simplified version of the problem).
type BoundActions<
TState,
TActions extends Record<string,  (...args : any[]) => (getState: () => TState, setState: (newState: Partial<TState>) => void, actions: BoundActions<TState,TActions>) => any>
> = {
[K in keyof TActions]: (
  ...args: Parameters<TActions[K]>
) => ReturnType<ReturnType<TActions[K]>>;
};

function createStoreSimplified<TState, TActions extends Record<string, (...args : any[]) => (getState: () => TState, setState: (newState: Partial<TState>) => void, actions: BoundActions<TState, TActions>) => any>>(initialState: TState, actions: TActions) : BoundActions<TState, TActions>{

  let state = initialState;

  const setState = (newPartialSate: Partial<TState>) => state = {...state, ...newPartialSate};
  const getState = () => state;

  const boundProps: BoundActions<TState, TActions> = {} as BoundActions<TState, TActions>;

  for (const key in actions) {
    if (actions.hasOwnProperty(key)) {
      const element = actions[key];
      boundProps[key] = (...args) => element(args)(getState, setState, boundProps);
    }
  }
  return boundProps
}

The following works
type State = {value: number}
const initialState: State = { value: 0};

type ActionsType = typeof actions;

const actions = {
  increment: (by : number = 1) => (getState: () => State, setState: (newState: Partial<State>) => void, actions: BoundActions<State, ActionsType>) => {
    setState({value : getState().value + by});
  },
  doSomethingAndIncrement: () => (getState: () => State, setState: (newState: Partial<State>) => void, actions: BoundActions<State, ActionsType>) => {
    actions.increment();
  },
}

const store1 = createStoreSimplified(initialState, actions);

store1.increment();

But when relying on type inference it does not work anymore:
const store2 = createStoreSimplified(initialState,{
  increment: () => (get, set, actions) => {
    set({value : get().value + 1});
  },
  doSomethingAndIncrement: () => (get, set, actions) => {
    actions.increment();
  },
});

store2.increment();

I fails with Property 'increment' does not exist on type 'BoundActions<State, unknown>'.ts(2339). Basically it considers actions as unknwown.
i tried a couple of things but couldn't find a way to fix this. Maybe someone has an idea?
The full project is https://github.com/atlassian/react-sweet-state

With #33050 the original example can now be written as:
interface A<a> {
    brand: 'a';
    nested: a;
}

interface B<a> {
    brand: 'b';
    nested: a;
}

type D = A<D> | B<D> | string;

I'm running into this limitation.
The suggested approach:
interface A<a> {
    brand: 'a';
    nested: a;
}

interface B<a> {
    brand: 'b';
    nested: a;
}

type D = A<D> | B<D> | string;

...pretty much works, though it leads to some verbosity in my code, because if you have a type like:
type Wrapper<T> = FooWrapper<T> | BarWrapper<T> | BazWrapper<T>

which you need for other purposes, you can't actually use it in a type like:
type Node = Wrapper<Node> | string

You have to write:
type Wrapper<T> = FooWrapper<T> | BarWrapper<T> | BazWrapper<T>;
type Node = FooWrapper<Node> | BarWrapper<Node> | BazWrapper<Node> | string;

The other issue I ran into with the interface workaround is that an interface can extend a tuple like [A, B, C], but not a tuple with a ... in it.  So I ended up playing musical chairs for a little while trying to understand the different type design constraints.


fixed missing format when the response schemas are extracted to types

This may not be the most appropriate way to fix the problem but hopefully the change/details below make it clear enough what issue I'm trying to address.
The issue is:

If extractResponseBody is set to true in the params the contentKind on the schema is being "lost".
That results in the format not being defined in the object constructed when sending a API call.
Because the format is missing the client will not attempt to decode the JSON data in the http client.

This behaviour differs to when extractResponseBody is false where format would be populated based on the API spec. IMO this is unexpected. It doesn't feel appropriate for this behaviour to differ based on the inline response schemas are extracted to named types or not.
... thanks for the prompt merge on my earlier MR as well. :)

It seems like the duplicate PR has already been opened: https://github.com/acacode/swagger-typescript-api/pull/742

Ah, missed it, yep. Thanks for pointing it out.


lib/game: ReaderWriter implementation, and some helpers

This PR implements the ReaderWriter interface, so that a Universe can be loaded or saved. It also adds functions universe.Dead() and universe.Size() to make the syntax a little more appealing.

Thanks for review @acifani now merging.


Add Animated Chips and cards demos

A couple of demos were added for showing animation transitions in Card and Chips
Checklist :clipboard:

[ ] Add explicit visibility modifier and explicit return types for public declarations
[ ] Run spotless check
[ ] Run tests
[ ] Update metalava's signature text files

Let's see if we can add AnimatedLabel into Horologist as a usable utility composable, rather than just a sample.


Compatibility issue with unstable branch

Indeed - you need to upgrade to the new type registration API. Should take less than 1m.

Would be happy to make a PR ;)

@itamarhaber, that would be great :)


The Windows.UI.Color type has no fields in ref assembly

Describe the bug

The Windows.UI.Color type in the System.Runtime.WindowsRuntime ref assembly shipped with the UWP SDK is incorrecty trimmed and contains no fields. This causes the C# compiler to allow methods that have an out Color parameter and no definite assignment to it. Given that the type actually does have fields, this technically breaks the C# spec and can result in incorrect behavior for users of the method. Essentially, this is like if a dev used Unsafe.SkipInit(out color) every time.
Noticed this while reviewing https://github.com/windows-toolkit/WindowsCommunityToolkit/pull/3503.
See comment here.
EDIT: apparently this is/was an issue with pretty much all .NET Standard 2.0 ref assemblies, see: https://github.com/dotnet/runtime/issues/16402.
Leaving this open (unless you want to close it) just as a reference until we can confirm this is no longer an issue with WinUI 3 😄
Steps to reproduce the bug

Steps to reproduce the behavior:

Add using Windows.UI
Paste the following snippet:

static void Test(out Color color)
{
}

Build

Expected behavior

The build should fail with the following error:

The out parameter 'color' must be assigned to before control leaves the current method

Screenshots

Here is the Color type opened in dotPeek:

Color type in dotPeek (click to expand):

Color type in dnSpy (click to expand):

The properties are empty as expected (ref assembly), but the fields being stripped away as well are causing the compiler to consider the type as an empty struct, and therefore allowing this method to build. There should be at least one field to block this behavior.
Version Info

SDK-style project using uap10.0.17763
Microsoft.NETCore.UniversalWindowsPlatform 6.2.9

Windows 10 version
Saw the problem?

Insider Build (xxxxx)

May 2020 Update (19041)
Yes

November 2019 Update (18363)

May 2019 Update (18362)

October 2018 Update (17763)

April 2018 Update (17134)

Fall Creators Update (16299)

Creators Update (15063)

Device form factor
Saw the problem?

Desktop
Yes

Xbox

Surface Hub

IoT

Confirmed that the issue repros in a UWP app, but not in a WinUI 3 app.