rtfeldman / elm-css

Typed CSS in Elm.
https://package.elm-lang.org/packages/rtfeldman/elm-css/latest
BSD 3-Clause "New" or "Revised" License
1.23k stars 196 forks source link

Add toNonceUnstyled functions for CSP nonce support #570

Closed jfmengels closed 2 years ago

jfmengels commented 2 years ago

Fixes #569

Happy to make any changes, and to take suggestions for better documentation of the function :slightly_smiling_face:

jonathanmoregard commented 2 years ago

Anything we can do to help get this in @rtfeldman ? :)

JohanWinther commented 2 years ago

@jfmengels For future-proofing the "API", do you think having a toUnstyledWith : Options -> Html msg -> VirtualDom.Node msg would be a good idea?

jfmengels commented 2 years ago

@JohanWinther That depends on what Options is. If it's a record, you'd call it like

Html.toUnstyledWith
  { nonce = Just "nonce id"
  ,  -- ...
  }
  (view model)

I think the API looks okay in that case, but it is not future-proof, because any addition/change you make to that record is a breaking change.

To make it future-proof, you can use the builder pattern:

Html.toUnstyledWith
  (Html.defaultOptions
    |> Html.withNonce "nonce id"
  )
  (model view)

The ergonomics become quite different, and not necessarily idiomatic to the rest of the package. In addition, the Html.Styled module would have a few additional out-of-place functions, or you move the options to a separate module.

The Html List pattern, with toUnstyledWith : List Option -> Html msg -> VirtualDom.Node msg, probably makes a lot more sense here:

Html.toUnstyledWith
  [ Html.nonce "nonce id"
  ]
  (model view)

But then the question becomes: What do we need to future proof for? I haven't followed feature requests for this repo enough, but this seems to be the first option since last the few years that elm-css has been released. So my gut feeling is that this is premature configuration and that we should wait until such a use-case comes around (or that someone mentions an existing one in thread). At that point, we can duplicate the toUnstyled functions for all possible permutations, or add one with options. A breaking change is also possibleand not necessarily out of question.

Anyway. If there is no use-case for a list of options, I don't think it's worth adding. But happy to add it if maintainers wish for it.

JohanWinther commented 2 years ago

@jfmengels I think you are on to something about not needing to future proof. I was looking at the way elm-ui handles it where a new option for the nonce was the way it was solved there, but of course, like you said, this is the first option in this package.

I'm looking forward to this change, mainly because I'm working at the same company (but different team) as @jonathanmoregard and my team will probably have the same problem to solve in the near future.

choonkeat commented 2 years ago

I think I'm missing something while trying to use Html.Styled.toNonceUnstyled. I'm still getting

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“style-src”)

and traced it down by modifying the elm output js

    for (var key in attrs)
    {
        var value = attrs[key];
+       if (key === 'style') {
+           console.log(value !== 'undefined' ? 'setAttribute' : 'removeAttribute', key, value, domNode);
+           continue;
+       }
        typeof value !== 'undefined'
            ? domNode.setAttribute(key, value)
            : domNode.removeAttribute(key);

to see this single line logged

Screenshot 2022-06-18 at 4 45 34 PM

which is from https://github.com/rtfeldman/elm-css/pull/559 https://github.com/rtfeldman/elm-css/blob/c5ffa8c2ec47d7822ce91aeb268a2dd3d398c72f/src/Css/Global.elm#L108-L111 https://github.com/rtfeldman/elm-css/blob/c5ffa8c2ec47d7822ce91aeb268a2dd3d398c72f/src/VirtualDom/Styled.elm#L483-L486

How do I avoid that?

_NOTE: I have .elm-css-style-wrapper { display: none; } in my css file, so would not need the inline style attribute_

mostlyobvious commented 2 years ago

According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src#violation_cases, inline style attributes (like in <span style="display: none;" class="elm-css-style-wrapper">...</span>) also violate style-src policy which does not allow unsafe-inline.

Current Html.Styled.toNonceUnstyled implementation is not enough as @choonkeat pointed out.