# Analysis of Elm core issues and patches

**URL:** <https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941>\
**Category:** Show and Tell\
**Created:** [November 26, 2021, 5:34pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941 "2021-11-26T17:34:14Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [November 26, 2021, 5:34pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/1 "2021-11-26T17:34:14Z")

</div>

Original sheet for gathering issues is here. This is editable if you want to add any more:

> **[Elm Essential Bug Fixes Assessment](https://docs.google.com/spreadsheets/d/1ICrw1EJVDNcXaxquGLXFmOLlDMsLHw7x5ltvrnf_TNg/edit?usp=sharing)**
>
> Sheet1
> 
> Description,GitHub Link,Ellie
> int parser fails on trailing decimal separator #14,https://github.com/elm/parser/issues/14 
> Url only supports http and https which prevents usage with protocols like ipfs:// and many, many other protocols....

I have analyzed what was posted in the original sheet, and started looking through the pull requests. Output of the analysis being prepared here:

> **[Elm Essential Bug Fixes Analysis](https://docs.google.com/spreadsheets/d/1vM56DhkNGPAknngrsCGqB6B9gjey1fpUg9TKZkGq5SQ/edit?usp=sharing)**
>
> Sheet1
> 
> Description,Area,GitHub Issue,Pull Request,Ellie or SSCCE or Demo etc,Pure Fix (Optimisation or Tail Recursion or Docs),Runtime Exception,Change to API Behaviour,API Addition (Backwards Compatible),API Change (Breaking),Change to Language...

---

<div class="post-metadata">

**Author:** ![matt.cheely](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/matt.cheely/32/4524_2.png) [@matt.cheely](https://discourse.elm-lang.org/u/matt.cheely)\
**Post date:** [November 27, 2021, 1:01am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/2 "2021-11-27T01:01:06Z")

</div>

I was looking through these and one thing that might be worth adding to the analysis is a “fixable in external package” check. As an example, `elm/parser`'s missing `deadEndsToString` doesn’t use any private information that I can see, so it would be feasible to implement it in a `parser-extras` or `parser-patch` library, and share that through the community. I suspect that could be true for most of these that don’t involve bugs in native code or re-implementation of operators.

---

<div class="post-metadata">

**Author:** ![lucamug](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/lucamug/32/1764_2.png) [@lucamug](https://discourse.elm-lang.org/u/lucamug)\
**Post date:** [November 27, 2021, 1:27am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/3 "2021-11-27T01:27:29Z")

</div>

Not sure if I remember correctly, but is not the `modBy 0 1` runtime exception an expected behavior? I think this was left as it is for performances reason. If performance is not needed, it is possible to wrap it in a `safeModBy` that adds a check for the parameter to be non-zero.

```elm
safeModBy : Int -> Int -> Maybe Int
safeModBy a b =
    if a == 0 then
        Nothing

    else
        Just (modBy a b)

```

Maybe `unsafeModBy` could be a better name for `modBy`?

It would be interesting to prioritize issues based on several factors. For example: do they have a workaround? How expensive is it? How popular is the package/function with the issue?

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [November 27, 2021, 9:39am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/4 "2021-11-27T09:39:56Z")

</div>

> [@matt.cheely](#):
>
> I was looking through these and one thing that might be worth adding to the analysis is a “fixable in external package” check.

That is sort of the function of the **Change to API Behaviour** and **API Addition** checks - changes to a core API might be one way to fix those, but external package might be a better way, especially if the core API is arguably not broken in the first place.

This one is a good example of that; is not allowing `4.` to parse as an Int a bug or a feature?

> <https://github.com/elm/parser/issues/14>
>
> Hi,
> I have found that the \`int\` parser fails on the input \`"42."\`. I would have… expected that \`int\` parses the \`"42"\`, and leaves \`"."\` to be consumed later on.
> 
> Here is an Ellie demonstrating the problem: https://ellie-app.com/3gJS4VwRJrfa1
> 
> If this is not a bug, I would propose to add documentation describing that behavior and how to work around it.

`deadEndsToString` would be fine in an external package, if it were not for the fact that it already exists in `elm/parser` and throws a runtime exception. Fixing it in an external package would still leave that broken code in place.

But yes, good idea, I will add a column for **Fixable in Non-Core Package**.

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [November 27, 2021, 9:46am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/5 "2021-11-27T09:46:15Z")

</div>

> [@lucamug](#):
>
> It would be interesting to prioritize issues based on several factors. For example: do they have a workaround? How expensive is it? How popular is the package/function with the issue?

My intention is to order them by impact of the fix on the rest of the system. Things that can be fixed in isolation are the lowest hanging fruit. So roughly in order of how I arranged the checks in the spreadsheet:

1. Pure Fix (Optimisation or Tail Recursion or Docs)
2. Runtime Exception
3. Change to API Behaviour
4. API Addition (Backwards Compatible)
5. API Change (Breaking)
6. Change to Language Semantics

But yes, you could play around with the order based on a number of other factors too.

---

<div class="post-metadata">

**Author:** ![malaire](https://avatars.discourse-cdn.com/v4/letter/m/b782af/32.png) [@malaire](https://discourse.elm-lang.org/u/malaire)\
**Post date:** [November 28, 2021, 4:35am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/6 "2021-11-28T04:35:26Z")

</div>

> [@rupert](#):
>
> This one is a good example of that; is not allowing `4.` to parse as an Int a bug or a feature?

You could ask if that is bug or feature, but what about `4d` parsing as Int but `4e` not? It’s quite hard to argue that not allowing `e` is a feature.

> <https://github.com/elm/parser/issues/28>
>
> \`int\` parser fails to parse \`42efg\` while parsing \`42abc\` works.
> ellie: https:/…/ellie-app.com/4jqm8pFGXyLa1
> 
> Related issues: https://github.com/elm/parser/issues/14 and https://github.com/elm/parser/issues/25

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [November 28, 2021, 10:33am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/7 "2021-11-28T10:33:24Z")

</div>

> [@malaire](#):
>
> You could ask if that is bug or feature, but what about `4d` parsing as Int but `4e` not? It’s quite hard to argue that not allowing `e` is a feature.

Lets add it to the sheet, looks fixable. 👍

---

<div class="post-metadata">

**Author:** ![miniBill](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/minibill/32/800_2.png) [@miniBill](https://discourse.elm-lang.org/u/miniBill)\
**Post date:** [November 30, 2021, 6:27pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/8 "2021-11-30T18:27:21Z")

</div>

I think line 32 and 14 are basically the same thing?

Excellent work with the analysis!

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [December 3, 2021, 6:36pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/9 "2021-12-03T18:36:02Z")

</div>

Analyzed all of the open PRs on elm/core now:

> **[Elm Essential Bug Fixes Analysis](https://docs.google.com/spreadsheets/d/1vM56DhkNGPAknngrsCGqB6B9gjey1fpUg9TKZkGq5SQ/edit?usp=sharing)**
>
> Sheet1
> 
> Description,Area,GitHub Issue,Pull Request,Ellie or SSCCE or Demo etc,Pure Fix (Optimisation or Tail Recursion or Docs),Runtime Exception,Change to API Behaviour,API Addition (Backwards Compatible),API Change (Breaking),Change to Language...

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [December 5, 2021, 5:14pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/10 "2021-12-05T17:14:32Z")

</div>

Added all the open PRs from elm/virtual-dom

Feel like I should add 2 more check box columns to the sheet.

1. For claimed performance improvements, as these should really be backed up with some benchmarks, in some cases its not really clear if a PR for a claimed perf improvement is really going to be a perf improvement.

2. For PRs that have raised some controversy in the discussion. Sometimes Evan might have commented to explain why he does not think something is a good idea, for example. Just to help flag up things that might seem straightforward but really need to be given some careful consideration.

> **[Elm Essential Bug Fixes Analysis](https://docs.google.com/spreadsheets/d/1vM56DhkNGPAknngrsCGqB6B9gjey1fpUg9TKZkGq5SQ/edit?usp=sharing)**
>
> Sheet1
> 
> Description,Area,GitHub Issue,Pull Request,Ellie or SSCCE or Demo etc,Pure Fix (Optimisation or Tail Recursion or Docs),Runtime Exception,Change to API Behaviour,API Addition (Backwards Compatible),API Change (Breaking),Change to Language...

---

<div class="post-metadata">

**Author:** ![Latty](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/latty/32/130_2.png) [@Latty](https://discourse.elm-lang.org/u/Latty)\
**Post date:** [December 5, 2021, 10:57pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/11 "2021-12-05T22:57:42Z")

</div>

On CSS custom properties, you can find the argument for inclusion here: [CSS Custom Properties](https://discourse.elm-lang.org/t/css-custom-properties/5554)

This is a vital feature for compatibility with web components and other modern web features, this issue is only going to get more and more important and painful as time goes on.

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [December 6, 2021, 10:14am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/12 "2021-12-06T10:14:18Z")

</div>

The fix seems harmless to me, in that it only allows new behaviour to be possible without breaking the old. In that sense, it does seem like the sort of thing we can and should fix.

I marked it as controversial simply because Evan weighed in with the opinion that its not how he envisages CSS being done in Elm (you would write a function in Elm to generate the CSS you want). When it comes to patching stuff in the core independantly of Evan, these are the kinds of issues that need careful consideration so I am glad you brought it up.

Do you have an opinion on how the fix might affect performance? Fix is to change:

`domNodeStyle[key] = styles[key];`

to

`domNodeStyle.setProperty(key, styles[key]);`

I know that `mdgriffith/elm-ui` has put a lot of thought into making its CSS very fast, and I am wary of upsetting that with this change. There is a UI benchmark somewhere that compares `elm-ui`, `elm-css` and a few other ways of styling isn’t there? Would be worth checking against that.

I’m not enough of a CSS expert to know if `.setProperty` is going to be slower.

---

<div class="post-metadata">

**Author:** ![lydell](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/lydell/32/178_2.png) [@lydell](https://discourse.elm-lang.org/u/lydell)\
**Post date:** [December 6, 2021, 10:30am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/13 "2021-12-06T10:30:38Z")

</div>

Regarding `.setProperty`, there’s some unfortunate backwards compatibility to consider too: [Allow CSS custom properties by using style.setProperty() by harrysarson · Pull Request #127 · elm/virtual-dom · GitHub](https://github.com/elm/virtual-dom/pull/127#issuecomment-803469849)

---

<div class="post-metadata">

**Author:** ![Latty](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/latty/32/130_2.png) [@Latty](https://discourse.elm-lang.org/u/Latty)\
**Post date:** [December 6, 2021, 12:55pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/14 "2021-12-06T12:55:32Z")

</div>

It may well be that exact implementation isn’t the best solution, if it makes more sense to have a special `setCustomProperty` that can only be used for that job to separate the implementation or some other alternate solution, that’s reasonable.

My focus is that the feature needs to be surfaced otherwise Elm is going to end up making any modern webcomponent unusable, and blocking access to other modern features (e.g: Houdini), I’m not advocating for any specific solution to surfacing it.

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [December 13, 2021, 4:51pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/15 "2021-12-13T16:51:58Z")

</div>

The spreadsheet has now been completed to cover all open Pull Requests on all core packages and the compiler.

Still happy to add any more issues that anyone wants to bring to my attention?

Other than that, that is as much as I am going to add to it for now. Next step will be to order things by level of impact to reveal what can be fixed without breaking anything versus changes that have deeper impacts that are not backwards compatible, or alter the language semantics. Then start a new thread with a summary of the spreadsheet around that topic.

> **[Elm Essential Bug Fixes Analysis](https://docs.google.com/spreadsheets/d/1vM56DhkNGPAknngrsCGqB6B9gjey1fpUg9TKZkGq5SQ/edit?usp=sharing)**
>
> Sheet1
> 
> Description,Area,GitHub Issue,Pull Request,Ellie or SSCCE or Demo etc,Pure Fix (Optimisation or Tail Recursion or Docs),Runtime Exception,Change to API or Runtime Behaviour,API Addition (Backwards Compatible),API Change (Breaking),Change to...

---

<div class="post-metadata">

**Author:** ![andre-dietrich](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/andre-dietrich/32/4106_2.png) [@andre-dietrich](https://discourse.elm-lang.org/u/andre-dietrich)\
**Post date:** [December 14, 2021, 9:25pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/16 "2021-12-14T21:25:37Z")

</div>

Maybe this can be added as an additional bug/fix … As I found, elm-apps crash when screen-readers are used:

> <https://github.com/elm/browser/issues/121>
>
> I just found out that the application of ScreenReaders for blind, such as the Ch…rome ScreenReaders results in a crash of an elm application. It seems to add additional nodes into the application, that elm cannot handle.
> 
> Tested with: https://chrome.google.com/webstore/detail/screen-reader/kgejglhpjiefppelpmljglcjbhoiplfn
> 
> !\[grafik\](https://user-images.githubusercontent.com/3089101/108722645-ac4d8a00-7523-11eb-969a-7a6c7449b49d.png)
> 
> It seems, that this problem can be fixed by applying the following patch:
> 
> https://github.com/jinjor/elm-break-dom
> 
> Probably this could be integrated in general, since it seems to solve a couple of other problems with browser extensions.

the same topic was also discussed here:

> [@Elm app not compatible with ChromeVox screen reader](https://discourse.elm-lang.org/t/elm-app-not-compatible-with-chromevox-screen-reader/3682):
>
> When I try to run my Elm app with [ChromeVox](https://chrome.google.com/webstore/detail/chromevox/kgejglhpjiefppelpmljglcjbhoiplfn?hl=en), I see a continual stream of “Uncaught TypeError: Cannot read property ‘childNodes’ of undefined”. What I’m guessing is I’m hitting a case similar to [what is described here](https://discourse.elm-lang.org/t/fullscreen-elm-app-in-0-19-childnode-issue-reopened/3174): i.e. it’s caused by Elm taking over the body. Do I have any resolution to make my app compatible with screen readers?

Additionally, the same error occurs, when googletranslate is used. All systems change the dom slightly, however this patch seems to work and solves the problem …

> **[GitHub - jinjor/elm-break-dom](https://github.com/jinjor/elm-break-dom)**
>
> Contribute to jinjor/elm-break-dom development by creating an account on GitHub.

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [December 15, 2021, 2:37pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/17 "2021-12-15T14:37:24Z")

</div>

> [@andre-dietrich](#):
>
> Maybe this can be added as an additional bug/fix … As I found, elm-apps crash when screen-readers are used:

Added it. I think the issue was already covered under the item “Browser extensions messing with the VDOM”, but no harm in having variations on it.

I marked this one as “API Change (Breaking)” as maybe we might need to change the Browser.application function type, so it accepts some further parameters that tell it where to put the Elm DOM. That isn’t the only way this could be fixed though, there are more backwards compatible ways it could be tackled.

---

<div class="post-metadata">

**Author:** ![andre-dietrich](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/andre-dietrich/32/4106_2.png) [@andre-dietrich](https://discourse.elm-lang.org/u/andre-dietrich)\
**Post date:** [December 16, 2021, 9:41am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/18 "2021-12-16T09:41:02Z")

</div>

Hi, maybe not the function-type, the patch only adds some checks to the function itself. It does not look very complex from my perspective:

> <https://github.com/jinjor/elm-break-dom/blob/master/patch/VirtualDom.patch>

---

<div class="post-metadata">

**Author:** ![lydell](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/lydell/32/178_2.png) [@lydell](https://discourse.elm-lang.org/u/lydell)\
**Post date:** [December 16, 2021, 11:43am UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/19 "2021-12-16T11:43:20Z")

</div>

> [@rupert](#):
>
> I marked this one as “API Change (Breaking)” as maybe we might need to change the Browser.application function type, so it accepts some further parameters that tell it where to put the Elm DOM. That isn’t the only way this could be fixed though, there are more backwards compatible ways it could be tackled.

FYI: Just rendering inside a `<div>` instead of directly inside `<body>` is not enough to fix the issue. Something in the spirit of the patch linked to above is needed.

---

<div class="post-metadata">

**Author:** ![rupert](https://yyz1.discourse-cdn.com/flex035/user_avatar/discourse.elm-lang.org/rupert/32/1775_2.png) [@rupert](https://discourse.elm-lang.org/u/rupert)\
**Post date:** [December 16, 2021, 1:06pm UTC](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941/20 "2021-12-16T13:06:08Z")

</div>

> [@lydell](#):
>
> FYI: Just rendering inside a `<div>` instead of directly inside `<body>` is not enough to fix the issue. Something in the spirit of the patch linked to above is needed.

I see. I didn’t actually see the patch since I didn’t go so far as digging into the `jinjor/elm-break-dom` project to look for it. I wonder why this patch was never submitted as a PR to `elm/virtual-dom`? Perhaps the author thought there was no point as it would not be merged?

Only comment I could find from Evan on the issue is:

[https://github.com/elm/html/issues/44#issuecomment-233145854](https://github.com/elm/html/issues/44#issuecomment-233145854)

> In the past, people (I _believe_ including [@rtfeldman](https://github.com/rtfeldman)) expressed frustration with having that additional named div in there, so I took it out. So I’m surprised by this recommendation. I’m not sure what to do given the history of this.

(the extra div approach screws up printer layouts apparently)

I will change the status of this one to “Runtime Exception” - seems like the patch could be a straightforward bug fix and a really helpful one too.

==============

Found some more from Evan:

> [@Fullscreen Elm App in 0.19 (childNode issue) Reopened](https://discourse.elm-lang.org/t/fullscreen-elm-app-in-0-19-childnode-issue-reopened/3174/20):
>
> For some additional context: I think adding things at the end of the \<body\> should be fine as is. I think it is possible to add some logic in virtual-dom to make it safe to add things at the beginning of \<body\> as well. Not 100% certain, but there is hope. So if the extensions are all in these two categories, we may be able to just have things work out. If the extensions are adding things in the middle of stuff, I’m not sure why they’d be doing it in the \<body\> any more than they’d be doing …

Possibly that post got jinjor started on compiling a list of problems and coming up with a fix. We just never completed the process and got the fix out…

[Next page](https://discourse.elm-lang.org/t/analysis-of-elm-core-issues-and-patches/7941.md?page=2)