Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/switch strict mode to true #127

Merged
merged 21 commits into from
Jul 30, 2021
Merged

Conversation

arnevandoorslaer
Copy link

No description provided.

@arnevandoorslaer arnevandoorslaer marked this pull request as ready for review July 27, 2021 07:55
Copy link
Member

@BelgianNoise BelgianNoise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we change to n3 Quads ?

@@ -9,21 +9,21 @@ export class InputComponent extends BaseComponent {
* The input field.
*/
@query('#content')
content: HTMLInputElement;
content!: HTMLInputElement;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this do exactly? ( just curious )

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is to make sure the variables are never null or undefined
https://lit.dev/docs/components/shadow-dom/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it actually prevents TS from doing the type check. In the case of LitElement, it might be semi-safe, since you "can be sure" that the template contains the element (if you're confident that you never make mistakes, which I am not of myself, for example)... but it will lead to runtime errors when trying to access the element before rendering, or when using it wrongly with state (see other comment).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to take a look at this again and decide what (and why) we want to do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, @arnevandoorslaer, I forgot about this... can you take another look at it tomorrow? I'd actually prefer to never use that syntax, and simply check for undefined values when they're used (explicitly or using ?. or ?? operators), but maybe you can convince me if you really think they have a huge advantage.

packages/semcom-node/tsconfig.json Outdated Show resolved Hide resolved
Co-authored-by: Arthur Joppart <38424924+BelgianNoise@users.noreply.github.com>
@arnevandoorslaer
Copy link
Author

Why did we change to n3 Quads ?

rdf-quads didn't have typings

packages/semcom-sdk/tsconfig.json Outdated Show resolved Hide resolved
packages/semcom-sdk/lib/addListener.ts Outdated Show resolved Hide resolved
packages/semcom-demo-app/lib/demo.component.ts Outdated Show resolved Hide resolved
packages/semcom-core/tsconfig.json Outdated Show resolved Hide resolved
packages/semcom-core/tsconfig.json Outdated Show resolved Hide resolved
packages/semcom-components/tsconfig.json Outdated Show resolved Hide resolved
packages/semcom-components/tsconfig.json Outdated Show resolved Hide resolved
@@ -9,21 +9,21 @@ export class InputComponent extends BaseComponent {
* The input field.
*/
@query('#content')
content: HTMLInputElement;
content!: HTMLInputElement;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it actually prevents TS from doing the type check. In the case of LitElement, it might be semi-safe, since you "can be sure" that the template contains the element (if you're confident that you never make mistakes, which I am not of myself, for example)... but it will lead to runtime errors when trying to access the element before rendering, or when using it wrongly with state (see other comment).

Arne Vandoorslaer and others added 4 commits July 27, 2021 16:22
@arnevandoorslaer arnevandoorslaer linked an issue Jul 29, 2021 that may be closed by this pull request
3 tasks
Signed-off-by: Wouter Termont <woutermont@gmail.com>
Signed-off-by: Wouter Termont <woutermont@gmail.com>
Signed-off-by: Wouter Termont <woutermont@gmail.com>
@woutermont woutermont merged commit 85a2c2d into develop Jul 30, 2021
@woutermont woutermont deleted the feat/switch-strict-mode-to-true branch July 30, 2021 09:59
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.

Switch to strict mode in TSconfigs
3 participants