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

Analysis-time errors should be halting #31

Closed
theengineear opened this issue May 7, 2019 · 3 comments · Fixed by #69
Closed

Analysis-time errors should be halting #31

theengineear opened this issue May 7, 2019 · 3 comments · Fixed by #69

Comments

@theengineear
Copy link
Collaborator

theengineear commented May 7, 2019

Currently, analysis work actually uses an instance (to resolve methods, etc). This allows us to dispatch errors at runtime instead of just throwing errors. Because we're not throwing halting errors, we're able to consider these errors non-fatal and move on.

In light of the comment below about the truly static observeAttributes hook and in light of discussions about making both computed methods and observers static #30, I propose we do cause a halting error.

It will:

  1. Remove forks in logic that deal with graceful failure
  2. Allow us to be truly static with our analysis
  3. Can't be missed by developers since it will fail to render anything

As long as analysis is truly static, you know that if an instance boots in development, it will boot in production, so it's not really possible to cause an error in production if you got the thing working in development.

@theengineear
Copy link
Collaborator Author

Here's an interesting thought related to analysis-time work. There does exist one-and-only-one hook in the HTMLElement class that you can depend on to be called before any this of an element sees the light of day.

🥁 - rull

static get observedAttributes!

I think this is actually pretty neat and leads me maybe change my mind about dispatching an error... I.e., I think maybe we should throw a halting error instead.

The observedAttributes hook is cool because it:

  1. Enforces that we don't have this yet, so we can't cheat even if we want to!
  2. Doesn't require any special hooks from base classes
  3. Can't be broken by subclasses since properties don't work at all if you don't call this on super since that's where we make sure reflected properties are observed.

I'm going to update the description to reflect this change in opinion, I believe a history of edits is kept now, so feel free to peruse that if you need the context.

@theengineear
Copy link
Collaborator Author

@klebba , you won me over on this one. Thanks for entertaining my bad idea 👍

@theengineear theengineear changed the title Discussion: Should analysis-time errors throw runtime errors or dispatch them instead? Analysis-time errors should be halting errors (not dispatched ones) May 10, 2019
@theengineear theengineear changed the title Analysis-time errors should be halting errors (not dispatched ones) Analysis-time errors should be halting May 10, 2019
@klebba
Copy link
Collaborator

klebba commented May 10, 2019

Hah, it was not a bad idea! Glad you were able to work through both options

theengineear added a commit that referenced this issue May 11, 2019
Eventually, we want analysis to be one-time, static work that is cached
per class. Therefore, we should not rely on having a target on which to
dispatch errors.

Closes #31
theengineear added a commit that referenced this issue May 11, 2019
Eventually, we want analysis to be one-time, static work that is cached
per class. Therefore, we should not rely on having a target on which to
dispatch errors.

Closes #31
theengineear added a commit that referenced this issue May 11, 2019
Eventually, we want analysis to be one-time, static work that is cached
per class. Therefore, we should not rely on having a target on which to
dispatch errors.

Closes #31
theengineear added a commit that referenced this issue May 30, 2020
theengineear added a commit that referenced this issue May 30, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 1, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 1, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 2, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 2, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 2, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 3, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 3, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 3, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 3, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 4, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 4, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 6, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 8, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 8, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 9, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 9, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 9, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jun 10, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 8, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 8, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 10, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 11, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 11, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 12, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 12, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 12, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 16, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 16, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 16, 2020
This closes #57, closes #52, closes #46, closes #42, closes #36,
closes #31, closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 16, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 17, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 17, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 19, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 19, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 19, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 19, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 22, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 22, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 22, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 22, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 24, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Jul 24, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Aug 24, 2020
This closes #57, closes #46, closes #42, closes #36, closes #31,
closes #30, closes #28, closes #26, and closes #25.
theengineear added a commit that referenced this issue Dec 9, 2020
This closes #68, closes #67, closes #66, closes #57, closes #46,
closes #42, closes #36, closes #31, closes #30, closes #28, closes #26,
and closes #25.
theengineear added a commit that referenced this issue Dec 9, 2020
This closes #68, closes #67, closes #66, closes #57, closes #46,
closes #42, closes #36, closes #31, closes #30, closes #28, closes #26,
and closes #25.
theengineear added a commit that referenced this issue Dec 9, 2020
This closes #68, closes #67, closes #66, closes #57, closes #46,
closes #42, closes #36, closes #31, closes #30, closes #28, closes #26,
and closes #25.
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 a pull request may close this issue.

2 participants