Skip to content

[fix]Add validation for Time component value prop#1331

Closed
SisyphusZheng wants to merge 1 commit intopreactjs:masterfrom
SisyphusZheng:patch-1
Closed

[fix]Add validation for Time component value prop#1331
SisyphusZheng wants to merge 1 commit intopreactjs:masterfrom
SisyphusZheng:patch-1

Conversation

@SisyphusZheng
Copy link
Contributor

@SisyphusZheng SisyphusZheng commented Sep 4, 2025

The Time component now checks if the 'value' prop is a non-empty string and logs warnings for invalid or unparsable values. This prevents rendering with invalid dates and improves error handling.

close #1334

The Time component now checks if the 'value' prop is a non-empty string and logs warnings for invalid or unparsable values. This prevents rendering with invalid dates and improves error handling.
@rschristian
Copy link
Member

Sorry, where did you run into this & what exactly does it solve? That component should only be used with our blog data which, as far as I can see, should all parse just fine?

If anything I'd rather have this throw & die as we should only be passing valid dates.

@SisyphusZheng
Copy link
Contributor Author

Sorry, where did you run into this & what exactly does it solve? That component should only be used with our blog data which, as far as I can see, should all parse just fine?

If anything I'd rather have this throw & die as we should only be passing valid dates.

So here's the thing: I've forked preact-www in a clean Windows environment, and I've run npm install and npm run dev. My original intention was just to translate v11, but I've encountered several minor errors that are preventing some web page content from functioning. I'll provide the error logs I've encountered later.

@rschristian
Copy link
Member

Strange. I don't doubt you at all, though it's unclear to me what could fail there as it all seems right. Would love to know more.

However, we absolutely don't want to be validating our own data. It should be correct or fail to build outright.

@rschristian
Copy link
Member

Closing as this only addresses a symptom of a problem shown in #1332, we don't want to actually be doing this.

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.

Invalid time value in Time component (index.jsx)

2 participants