chore(create-vite): use non-null assertion

[nickmccurdy]

Description

It's safer to use non-null assertions over unnecessary type assertions in TypeScript, since type assertions are bivariant.

-ReactDOM.createRoot(document.getElementById('root') as HTMLElement))
+ReactDOM.createRoot(document.getElementById('root')!)

Here are some practical advantages of using non-null assertions to render React elements:

• Readability of using the right syntax to assert a non-null value
• Discouraging use of as which many TypeScript beginners may not understand the dangers of
• You won't get a type error if you render to an Element that isn't an HTMLElement, such as <svg>/SVGSVGElement
• You will get a type error if you render to a value that isn't always an Element, such as Element | boolean

---

What is the purpose of this pull request?

• [ ] Bug fix
• [ ] New Feature
• [ ] Documentation update
• [x] Other

Before submitting the PR, please make sure you do the following

• [x] Read the Contributing Guidelines.
• [x] Read the Pull Request Guidelines and follow the Commit Convention.
• [x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• [ ] Ideally, include relevant tests that fail without this PR but pass with it.

[nickmccurdy]

Not sure if the CI test failure is related.

[benmccann]

any reason this PR is marked as draft?

Not sure if the CI test failure is related.

it's not. I re-ran it and it passed

[patak-dev]

This is intentional: https://github.com/vitejs/vite/pull/7881#issuecomment-1108410477

[nickmccurdy]

@patak-dev I'm aware of that, but as I explained, this is less safe and more confusing than using a non-null assertion. Can we please revert this?

Edit: This seems to be an issue with TypeScript ESLint's config. TypeScript ESLint has a rule that explains why a non-null assertion is better. However, it's unfortunately only enabled in the strict config. I'm in the process of discussing this with upstream maintainers.

[patak-dev]

Thanks Nick. Reopening while you discuss with them.

[JLarky]

now I feel responsible because I created that meme :) so I just want to add the second part of it to the discussion: you can also perform the runtime check which a lot of apps do do in production:

const root = document.getElementById('root')
if (!root) throw new Error('No root element found')

ReactDOM.createRoot(root).render(
  <React.StrictMode>
    <App />
  </React.StrictMode>
)

[ArnaudBarre]

There is already a runtime check in createRoot if passed null. I don't think this manual check is worth it.

I don't get why we the default rules disallow ! and not as. One is way more dangerous than the other, as explained by the rule that hint to use the former when the check is identical.

I will update the lint rules of the repo to add type ware rules and fix this at the same time

[nickmccurdy]

There is already a runtime check in createRoot if passed null. I don't think this manual check is worth it.

The error message is Target container is not a DOM element., the same one if I pass any other non-DOM value. So it's not specifically checking if this exist, though I agree the manual check still isn't super useful in this scenario, as we control the HTML. Ultimately the issue is that ! should be allowed with nullable values.

I will update the lint rules of the repo to add type ware rules and fix this at the same time

You mean you're adding ESLint configs to the create-vite templates? 😮 I'd like that. Let me know if you'd like help.

[ArnaudBarre]

My work on this is in pause for now, but maybe I could just try to change this one rule for this use-case before the next stable so that the 4.1 template uses !

[nickmccurdy]

Would it help if I tried to patch this in TypeScript ESLint's recommended config? Then we could just use that until the Vite specific lint config is ready (though I think their defaults are really good, excluding the ! ban).

[ArnaudBarre]

I think they don't want to include type information for default rules.

I not sure I will have time to do this today, so you can open a PR that just turn on this rule for the Vite repo