Skip to content

web: fix value handling inside controlled components (cherry-pick #9648)#9685

Merged
BeryJu merged 1 commit intoversion-2024.4from
cherry-pick-16f5cb-version-2024.4
May 13, 2024
Merged

web: fix value handling inside controlled components (cherry-pick #9648)#9685
BeryJu merged 1 commit intoversion-2024.4from
cherry-pick-16f5cb-version-2024.4

Conversation

@gcp-cherry-pick-bot
Copy link
Contributor

Cherry-picked web: fix value handling inside controlled components (#9648)

  • web: fix esbuild issue with style sheets

Getting ESBuild, Lit, and Storybook to all agree on how to read and parse stylesheets is a serious
pain. This fix better identifies the value types (instances) being passed from various sources in
the repo to the three different kinds of style processors we're using (the native one, the
polyfill one, and whatever the heck Storybook does internally).

Falling back to using older CSS instantiating techniques one era at a time seems to do the trick.
It's ugly, but in the face of the aggressive styling we use to avoid Flashes of Unstyled Content
(FLoUC), it's the logic with which we're left.

In standard mode, the following warning appears on the console when running a Flow:

Autofocus processing was blocked because a document already has a focused element.

In compatibility mode, the following error appears on the console when running a Flow:

crawler-inject.js:1106 Uncaught TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'.
    at initDomMutationObservers (crawler-inject.js:1106:18)
    at crawler-inject.js:1114:24
    at Array.forEach (<anonymous>)
    at initDomMutationObservers (crawler-inject.js:1114:10)
    at crawler-inject.js:1549:1
initDomMutationObservers @ crawler-inject.js:1106
(anonymous) @ crawler-inject.js:1114
initDomMutationObservers @ crawler-inject.js:1114
(anonymous) @ crawler-inject.js:1549

Despite this error, nothing seems to be broken and flows work as anticipated.

  • web: fix value handling inside controlled components

This is one of those stupid bugs that drive web developers crazy. The basics are straightforward:
when you cause a higher-level component to have a "big enough re-render," for some unknown
definition of "big enough," it will re-render the sub-components. In traditional web interaction,
those components should never be re-rendered while the user is interacting with the form, but in
frameworks where there's dynamic re-arrangement, part or all of the form could get re-rendered at
any mmoment. Since neither the form nor any of its intermediaries is tracking the values as they're
changed, it's up to the components themselves to keep the user's input-- and to be hardened against
property changes coming from the outside world.

So static memoization of the initial value passed in, and aggressively walling off the values the
customer generates from that field, are needed to protect the user's work from any framework's
dynamic DOM management. I remember struggling with this in React; I had hoped Lit was better, but in
this case, not better enough.

The protocol for "is it an ak-data-control" is "it has a json() method that returns the data ready
to be sent to the authentik server." I missed that in one place, so that's on me.

  • Eslint had opinions.

  • Added comments to explain something.

* web: fix esbuild issue with style sheets

Getting ESBuild, Lit, and Storybook to all agree on how to read and parse stylesheets is a serious
pain. This fix better identifies the value types (instances) being passed from various sources in
the repo to the three *different* kinds of style processors we're using (the native one, the
polyfill one, and whatever the heck Storybook does internally).

Falling back to using older CSS instantiating techniques one era at a time seems to do the trick.
It's ugly, but in the face of the aggressive styling we use to avoid Flashes of Unstyled Content
(FLoUC), it's the logic with which we're left.

In standard mode, the following warning appears on the console when running a Flow:

```
Autofocus processing was blocked because a document already has a focused element.
```

In compatibility mode, the following **error** appears on the console when running a Flow:

```
crawler-inject.js:1106 Uncaught TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'.
    at initDomMutationObservers (crawler-inject.js:1106:18)
    at crawler-inject.js:1114:24
    at Array.forEach (<anonymous>)
    at initDomMutationObservers (crawler-inject.js:1114:10)
    at crawler-inject.js:1549:1
initDomMutationObservers @ crawler-inject.js:1106
(anonymous) @ crawler-inject.js:1114
initDomMutationObservers @ crawler-inject.js:1114
(anonymous) @ crawler-inject.js:1549
```

Despite this error, nothing seems to be broken and flows work as anticipated.

* web: fix value handling inside controlled components

This is one of those stupid bugs that drive web developers crazy. The basics are straightforward:
when you cause a higher-level component to have a "big enough re-render," for some unknown
definition of "big enough," it will re-render the sub-components. In traditional web interaction,
those components should never be re-rendered while the user is interacting with the form, but in
frameworks where there's dynamic re-arrangement, part or all of the form could get re-rendered at
any mmoment. Since neither the form nor any of its intermediaries is tracking the values as they're
changed, it's up to the components themselves to keep the user's input-- and to be hardened against
property changes coming from the outside world.

So static memoization of the initial value passed in, and aggressively walling off the values the
customer generates from that field, are needed to protect the user's work from any framework's
dynamic DOM management. I remember struggling with this in React; I had hoped Lit was better, but in
this case, not better enough.

The protocol for "is it an ak-data-control" is "it has a `json()` method that returns the data ready
to be sent to the authentik server."  I missed that in one place, so that's on me.

* Eslint had opinions.

* Added comments to explain something.
@gcp-cherry-pick-bot gcp-cherry-pick-bot bot requested a review from a team as a code owner May 10, 2024 18:57
@netlify
Copy link

netlify bot commented May 10, 2024

Deploy Preview for authentik-docs ready!

Name Link
🔨 Latest commit ba22e33
🔍 Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/663e6e03620b6600084b67dd
😎 Deploy Preview https://deploy-preview-9685--authentik-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented May 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.50%. Comparing base (9240fa1) to head (ba22e33).

Additional details and impacted files
@@               Coverage Diff               @@
##           version-2024.4    #9685   +/-   ##
===============================================
  Coverage           92.50%   92.50%           
===============================================
  Files                 669      669           
  Lines               32915    32915           
===============================================
  Hits                30447    30447           
  Misses               2468     2468           
Flag Coverage Δ
e2e 50.53% <ø> (-0.02%) ⬇️
integration 25.91% <ø> (ø)
unit 89.82% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

authentik PR Installation instructions

Instructions for docker-compose

Add the following block to your .env file:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-ghcr.io/goauthentik/dev-server:gh-ba22e33fc29bcca5902d463ae6cfa8194f7ac6ce
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

For arm64, use these values:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-ghcr.io/goauthentik/dev-server:gh-ba22e33fc29bcca5902d463ae6cfa8194f7ac6ce-arm64
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

Afterwards, run the upgrade commands from the latest release notes.

Instructions for Kubernetes

Add the following block to your values.yml file:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
    repository: ghcr.io/goauthentik/dev-server
    tag: gh-ghcr.io/goauthentik/dev-server:gh-ba22e33fc29bcca5902d463ae6cfa8194f7ac6ce

For arm64, use these values:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
    repository: ghcr.io/goauthentik/dev-server
    tag: gh-ghcr.io/goauthentik/dev-server:gh-ba22e33fc29bcca5902d463ae6cfa8194f7ac6ce-arm64

Afterwards, run the upgrade commands from the latest release notes.

@BeryJu BeryJu merged commit ade1f08 into version-2024.4 May 13, 2024
@BeryJu BeryJu deleted the cherry-pick-16f5cb-version-2024.4 branch May 13, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants