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

React: Server-fetched instant field remains string type #24435

Closed
1 task done
hide212131 opened this issue Dec 3, 2023 · 3 comments
Closed
1 task done

React: Server-fetched instant field remains string type #24435

hide212131 opened this issue Dec 3, 2023 · 3 comments

Comments

@hide212131
Copy link
Contributor

Overview of the issue

Related to #23769 (comment)

export interface IEmployee {
  hireDate?: dayjs.Dayjs | null;
}

export const EmployeeUpdate = () => {
  const employeeEntity = useAppSelector<IEmployee>(state => state.employee.entity);

  // If we write this expecting hireDate to be a dayjs type, we get an error at runtime. 
  // This is because hireDate can be a **string** type.
  const year = employeeEntity.hireDate?.year(); 

With the model as dayjs type, the results retrieved from the REST API must be converted.

Motivation for or Use Case

While this doesn't directly cause an error, it can lead to confusion during frontend development.

Reproduce the error
Related issues

#23769

Suggest a Fix

#23769 (comment)

JHipster Version(s)

https://github.com/jhipster/generator-jhipster/tree/3c5e8327f8909923a2411aeb20dcd0cbc1e96cfc

  • Checking this box is mandatory (this is just to show you read everything)
@qmonmert
Copy link
Contributor

@hide212131 I did what you suggested but I have a warning in the console
image
and it seems that it is not a good practice
https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions
https://redux.js.org/faq/organizing-state#can-i-put-functions-promises-or-other-non-serializable-items-in-my-store-state

cc @DanielFran

@hide212131
Copy link
Contributor Author

@qmonmert (cc @DanielFran) You're absolutely right, and I apologize for overlooking that.
I've also gone through this discussion. Given that we are using React + Redux-toolkit, it's recommended that our models are Serializable/Immutable. Unfortunately (as it is less usable than Angular or Vue), this means it would be better to revert to using the string type, specifically in the ISO 8601 format.

Additionally, if we decide to use the string type, we will need to change the role of src/main/webapp/app/shared/util/date-utils.ts.

From:

export const convertDateTimeToServer = (date?: string): dayjs.Dayjs | null => (date ? dayjs(date) : null);

To:

// React components deal with local time, so it's necessary to convert them to UTC.
export const convertLocalToUTC = (date?: string): string | null => (date ? dayjs(date).toISOString() : null);

The same applies to convertDateTimeFromServer.

Could you please provide your thoughts on whether this approach is appropriate? There's also the option of overriding the isSerializable option in the serialization check middleware.

Copy link
Contributor

github-actions bot commented Jul 1, 2024

This issue is stale because it has been open for too long without any activity.
Due to the moving nature of jhipster generated application, bugs can become invalid.
If this issue still applies please comment otherwise it will be closed in 7 days

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2024
@mraible mraible added this to the 8.7.0 milestone Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants