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

fix(table): reorder params from Object.assign to allow for non-extensible objects #689

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

tcitworld
Copy link
Contributor

Non-extensible objects (such as VueJS computed properties) don't allow to be extended with Object.assign(). By inverting the two parameters, we copy all the properties of the row to the new { __rowKey: uuid() } object instead.

Fixes TypeError: can't define property "__rowKey": Object is not extensible

…ible objects

Non-extensible objects (such as VueJS computed properties) don't allow to be extended with
Object.assign(). By inverting the two parameters, we copy all the properties of the row to the new
`{ __rowKey: uuid() }` object instead.

Signed-off-by: Thomas Citharel <[email protected]>
Copy link

netlify bot commented Dec 13, 2023

Deploy Preview for oruga-documentation-preview ready!

Name Link
🔨 Latest commit 5305334
🔍 Latest deploy log https://app.netlify.com/sites/oruga-documentation-preview/deploys/6579e108ced0b500089c2172
😎 Deploy Preview https://deploy-preview-689--oruga-documentation-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d8205ca) 51.93% compared to head (5305334) 51.93%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #689   +/-   ##
========================================
  Coverage    51.93%   51.93%           
========================================
  Files           33       33           
  Lines         1396     1396           
  Branches       515      515           
========================================
  Hits           725      725           
  Misses         671      671           
Flag Coverage Δ
oruga-next 51.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mlmoravek
Copy link
Member

@tcitworld Your change seems right to me, but I'll have a closer look in the next few weeks if I have the time :)

@mlmoravek mlmoravek self-assigned this Dec 18, 2023
@mlmoravek mlmoravek merged commit dc2b9cf into oruga-ui:develop Dec 18, 2023
8 checks passed
@tcitworld tcitworld deleted the fix-table-immutable-data branch December 18, 2023 13:43
@Nemesis19
Copy link

Nemesis19 commented Dec 21, 2023

if :data="data" is an array of typescript classes, these classes get somehow transformed into plain objects?

In my classes I have methods that turn to errors like "TypeError: row.mycustomfunction is not a function", when "mycustomfunction" is one of the methods inside the class.

You cannot use TS Classes at their full potential.

Rolling back to previous version solve the problem to me.

@mlmoravek
Copy link
Member

mlmoravek commented Dec 21, 2023

@Nemesis19 Could you explain your problem in more detail, maybe with some more example code? Should the methods of your typescript classes not be compiled to functions of a js object anyway?
Maybe open an issue to have a more transparent conversation about this.

@Nemesis19
Copy link

Nemesis19 commented Dec 21, 2023

example of my user class, the o-table gets as :data an array of this Typescript Classes.

The column where I'm using a class method method (row.data('something')) returns the error above.

import { Exclude, Type } from 'class-transformer';

export class User  {

	id: number;

	organizationId: number;

	name: string;
	surname: string;
	email: string;
	password?: string;
	passwordConfirmation?: string;

	@Exclude({ toPlainOnly: true })
	@Type(() => Organization)
	organization: Organization;

	@Exclude({ toPlainOnly: true })
	permissions: string[];

	@Exclude({ toPlainOnly: true })
	roles: string[];

	@Exclude({ toPlainOnly: true })
	@Type(() => Contact)
	contacts: Contact[];

	@Exclude({ toPlainOnly: true })
	@Type(() => QuestionnaireUser)
	questionnairesuser: QuestionnaireUser[];

@Transform(dateTransform)
	createdAt: Date;

	@Transform(dateTransform)
	updatedAt: Date;

	get fullname(): string {
		let fullname: string = this.name;
		if (this.surname) fullname += ` ${this.surname}`;
		return fullname;
	}

	get deleteLabel(): string {
		return `${User.SINGLE_ENTITY}<br><b>"${this.fullname}"</b>`;
	}

	get headerLabel(): string {
		return this.fullname;
	}

	get codiceFiscale(): string {
		if (!this.contacts?.length) return '';
		return this.contacts[0].cf as string;
	}

date(prop: string) {
		const format: any = {
			day: '2-digit', month: '2-digit', year: 'numeric'
		};
		if (this[prop]) return this[prop].toLocaleDateString('it-IT', format);
	}

	dateTime(prop: string, seconds?: boolean) {
		const format: any = {
			day: '2-digit', month: '2-digit', year: 'numeric',
			hour: 'numeric', minute: 'numeric'
		};
		if (seconds) format.second = '2-digit';
		if (this[prop]) return this[prop].toLocaleDateString('it-IT', format);
	}
}

example of o-table column, see {{ row.date('updatedAt') }}

<o-table
			striped
			backend-pagination
			:paginated="paginated"
			:data="data"
			:total="page.total"
			:current-page="page.current"
			:per-page="page.perPage"
			:order="'left'"
			@page-change="onPageChange">
			<o-table-column
				field="id"
				position="centered"
				label="ID"
				numeric
				v-slot="{ row }">
				{{ row.id }}
			</o-table-column>
			<o-table-column
				field="name"
				position="centered"
				label="Nome"
				v-slot="{ row }">
				{{ row.name }}
			</o-table-column>
			<o-table-column
				field="surname"
				position="centered"
				label="Cognome"
				v-slot="{ row }">
				{{ row.surname }}
			</o-table-column>
			<o-table-column
				field="organization.name"
				position="centered"
				label="Organizzazione"
				v-slot="{ row }">
				{{ row.organization?.name }}
			</o-table-column>
			<o-table-column
				field="email"
				position="centered"
				label="Email"
				v-slot="{ row }">
				{{ row.email }}
			</o-table-column>
			<o-table-column
				field="updatedAt"
				position="centered"
				label="Modificato"
				v-slot="{ row }">
				{{ row.date('updatedAt') }}
			</o-table-column>
			<o-table-column v-slot="{ row }">
				<ActionIcons
					@view="onView(row)"
					@edit="onEdit(row)"
					@delete="handleDelete(row)" />
			</o-table-column>
			<template #empty>
				<TableEmpty></TableEmpty>
			</template>
			<template
				v-if="paginated"
				#bottom-left>
				Totale: {{ page.total }}
			</template>
			<template
				v-if="!paginated"
				#footer>
				Totale: {{ page.total }}
			</template>
		</o-table>

the problem is definitely in this merge, cause till 0.8.1 everything was working as expected and honestly I don't understand what this merge really fixes and it's purpose since o-table was a piece of art 😅...but whatever...

It seems this update transforms a typescript class into a plain object, making the class to lose all the class features.

@mlmoravek
Copy link
Member

@Nemesis19 We agree this is at all a bad concept.
I opened a issue for this: #701

In the meantime, you might try implementing the specific function you want to call as a property with an anonymous function instead of a method. That way it is defined as a prop in the object. Perhaps this will solve your problem for now.

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.

3 participants