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

chore: extend root eslint config in twenty-server #4781

Closed

Conversation

thaisguigon
Copy link
Contributor

Following #4705: this is a proposal to extend the common root eslint config in twenty-server as well.

@thaisguigon thaisguigon force-pushed the chore/mutualize-eslint-config branch from a3c8448 to 89f73f0 Compare April 3, 2024 10:20
@thaisguigon thaisguigon changed the title Chore/use root eslint config in twenty server chore: extend root eslint config in twenty-server Apr 3, 2024
@thaisguigon thaisguigon force-pushed the chore/use-root-eslint-config-in-twenty-server branch 2 times, most recently from 4e3a6ec to 3e82703 Compare April 3, 2024 10:22
@thaisguigon thaisguigon requested a review from magrinj April 4, 2024 08:08
Base automatically changed from chore/mutualize-eslint-config to main April 4, 2024 10:05
@thaisguigon thaisguigon force-pushed the chore/use-root-eslint-config-in-twenty-server branch from 3e82703 to 0c0ea3b Compare April 4, 2024 14:14
@thaisguigon thaisguigon marked this pull request as ready for review April 4, 2024 14:33
@thaisguigon thaisguigon self-assigned this Apr 4, 2024
@thaisguigon thaisguigon assigned magrinj and unassigned thaisguigon Apr 4, 2024
Comment on lines -1 to -7
export function orderObjectProperties<T extends object>(data: T[]): T[];

export function orderObjectProperties<T extends object>(data: T): T;

export function orderObjectProperties<T extends Array<any> | object>(
export const orderObjectProperties = <T extends object[] | object>(
data: T,
): T {
Copy link
Member

Choose a reason for hiding this comment

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

@thaisguigon We should keep the use of function on the server side, only use arrow function is dropping multiple signature capability

@@ -31,7 +31,7 @@ export const createApp = async (
imports: [AppModule],
});

if (!!config.moduleBuilderHook) {
if (config.moduleBuilderHook) {
Copy link
Member

Choose a reason for hiding this comment

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

@thaisguigon Hmm careful with that, in this case it's not causing any issue, but when you're doing that in a react-native three it will cause error, it should not happen on react but can too:

This will cause issue (empty string is not allowed in the three):

<>
  {myString && <></>}
</>

But this is not causing issue:

<>
  {!!myString && <></>}
</>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This custom rule (explicit-boolean-predicates-in-if) only applies to if statements, it won't flag conditions in JSX.

@Weiko
Copy link
Member

Weiko commented Apr 22, 2024

To avoid the many conflicts, I've reopened a PR here #5101 and cherry picked the config change, will run the linter with the new folder structure and hopefully will merge today 🙏

@Weiko Weiko closed this Apr 22, 2024
charlesBochet pushed a commit that referenced this pull request Apr 22, 2024
Reopening @thaisguigon work from
#4781

---------

Co-authored-by: Thaïs Guigon <guigon.thais@gmail.com>
@charlesBochet charlesBochet deleted the chore/use-root-eslint-config-in-twenty-server branch June 29, 2024 09:36
arnavsaxena17 pushed a commit to arnavsaxena17/twenty that referenced this pull request Oct 6, 2024
Reopening @thaisguigon work from
twentyhq#4781

---------

Co-authored-by: Thaïs Guigon <guigon.thais@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants