Grid.js v6 - Functional Components #1267

afshinm · 2022-12-25T16:15:46Z

Grid.js v6 🚀

Context

This PR updates Grid.js and most internal components to work with Functional Preact components instead of class components. In addition to that, I have worked on simplifying the Config object and the state management process.

Notable changes are:

Refactored the Plugins and internal components to the Functional component form factor
Implemented a simple Redux library to manage the internal state (added useConfig, useContext and useStore)
Removed the centralised dispatcher object and replaced it with a central Store object
Updated the Container and Grid component to use Context for passing the Config object to downstream components
Simplified the Config object and removed the redundant/unused attributes
Updated and refactored the unit tests

Why?

Simplified Config and State management process along with Functional components (instead of class components) makes Grid.js much easier to maintain and manage. It also reduces the build output size by about 30% which is a great win.

Feedback

I understand this is a huge PR (15k lines changed) and I apologise. It's almost impossible to break down this PR into smaller change sets but I'm happy to provide more context if anyone has any questions.

github-actions · 2022-12-28T11:49:11Z

Size Change: -25.8 kB (-26%) 🎉

Total Size: 72 kB

Filename	Size	Change
`dist/gridjs.js`	11.8 kB	-5.28 kB (-31%)	🎉
`dist/gridjs.module.js`	11.8 kB	-5.33 kB (-31%)	🎉
`dist/gridjs.production.es.min.js`	11.8 kB	-5.33 kB (-31%)	🎉
`dist/gridjs.production.min.js`	11.9 kB	-5.22 kB (-30%)	🎉
`dist/gridjs.umd.js`	11.9 kB	-5.22 kB (-30%)	🎉
`l10n/dist/l10n.js`	3.05 kB	+197 B (+7%)	🔍
`l10n/dist/l10n.module.js`	3.13 kB	+204 B (+7%)	🔍
`l10n/dist/l10n.umd.js`	3.18 kB	+196 B (+7%)	🔍
`plugins/selection/dist/selection.js`	1.05 kB	-7 B (-1%)
`plugins/selection/dist/selection.module.js`	1.08 kB	+4 B (0%)
`plugins/selection/dist/selection.umd.js`	1.16 kB	-5 B (0%)

_{compressed-size-action}

github-actions · 2022-12-29T15:09:47Z

jest coverage report 🧪

❌ The test suite failed. Please, check the console output for more details.

abitbetterthanyesterday · 2023-01-02T21:27:51Z

Geez @afshinm you have been busy during the holidays!
I'll have a look through it and give you feedback.

abitbetterthanyesterday · 2023-01-02T21:29:27Z

index.ts

 h,
 createElement,
 Component,
 createRef,
 useEffect,
- useRef
+ useRef,


Why do we re-export preact's exports? I'm curious!

That's mainly for people who want to extend Grid.js or write Grid.js components. Note that when we compile Grid.js, Preact is already included in the output (i.e you don't have to install Preact separately).

src/config.ts

github-actions · 2023-01-03T10:42:19Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	79.93% (-0.66% 🔻)	1294/1619
🟡	Branches	66.54% (-1.91% 🔻)	346/520
🟢	Functions	81.05% (-0.61% 🔻)	278/343
🟡	Lines	79.58% (-0.79% 🔻)	1138/1430

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	hooks/useConfig.ts	100%	100%	100%	100%
🟢	hooks/useStore.ts	100%	100%	100%	100%
🟢	`...` / useSelector.ts	100%	100%	100%	100%
🟢	state/store.ts	90.91%	0%	100%	88.46%
🟢	view/actions.ts	96.55%	50%	100%	100%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / tbody.tsx	100%	92.31% (-7.69% 🔻)	100%	100%
🟢	view/table/td.tsx	84.62% (-3.85% 🔻)	69.23% (-5.77% 🔻)	100%	87.5% (-3.41% 🔻)
🟢	plugin.ts	80.43% (-3.2% 🔻)	76.47% (+7.24% 🔼)	100%	80.49% (-1.73% 🔻)
🟢	config.ts	97.5% (-0.33% 🔻)	85.71% (-4.29% 🔻)	100%	100%
🟢	tabular.ts	83.87% (-6.45% 🔻)	62.5% (-25% 🔻)	84.62%	85.19% (-7.41% 🔻)
🟢	`...` / eventEmitter.ts	93.55% (-6.45% 🔻)	87.5% (-12.5% 🔻)	87.5% (-12.5% 🔻)	93.1% (-6.9% 🔻)
🟡	`...` / pagination.tsx	81.01% (-9.46% 🔻)	62.22% (-17.19% 🔻)	55.56% (-38.19% 🔻)	78.79% (-10.87% 🔻)
🟡	header.ts	77.27% (-2.02% 🔻)	67.9% (-1.87% 🔻)	88.89% (-5.85% 🔻)	77.44% (-2.01% 🔻)
🔴	`...` / shadow.tsx	12.12% (-10.38% 🔻)	0%	0% (-14.29% 🔻)	12.12% (-6.8% 🔻)
🟡	view/table/th.tsx	78.72% (-3.28% 🔻)	68.75% (-2.22% 🔻)	75% (-6.82% 🔻)	78.57% (-4.04% 🔻)
🟡	`...` / sort.tsx	76.47% (+0.66% 🔼)	44% (-3.83% 🔻)	81.82% (+26.26% 🔼)	75.44% (-0.83% 🔻)
🔴	`...` / actions.ts	8.16% (-69.61% 🔻)	0% (-100% 🔻)	0% (-33.33% 🔻)	8.7% (-62.73% 🔻)
🔴	`...` / resize.tsx	12.9% (-23.21% 🔻)	0%	0% (-14.29% 🔻)	13.33% (-19.02% 🔻)
🟢	view/container.tsx	100%	91.67% (-0.64% 🔻)	100%	100%
🟡	grid.ts	72.22% (-0.51% 🔻)	28.57%	71.43% (-11.9% 🔻)	71.43% (-0.45% 🔻)
🔴	util/getConfig.ts	0% (-87.5% 🔻)	0% (-50% 🔻)	0% (-100% 🔻)	0% (-100% 🔻)

Test suite run success

176 tests passing in 31 suites.

Report generated by 🧪jest coverage report action from 95d8564

abitbetterthanyesterday

Given the limited time I had to review this, there is too much to absorb, but thank you for the work. Great job.

It should make contributing much easier in the future.

💪 👏 🍺

src/header.ts

abitbetterthanyesterday · 2023-01-09T22:27:01Z

src/view/plugin/resize/resize.tsx

+ document.removeEventListener('touchend', end);
+ };
+
+ return (


There is a lot going on here so I haven't fully absorbed the whole change, but it might be worth considering wrapping the mouse event in useCallback to memoize them? I had performance issues in the past using mouse listeners.

It might be unnecessary here, I don't have a clear picture of the code logic.

abitbetterthanyesterday · 2023-01-09T22:28:01Z

src/view/plugin/search/search.tsx

@@ -1,16 +1,18 @@
-import { h } from 'preact';
+import { h, JSX } from 'preact';


These changes might fix #1081

abitbetterthanyesterday · 2023-01-09T22:31:02Z

src/view/table/th.tsx

+ const [style, setStyle] = useState({});
+ const { dispatch } = useStore();
+
+ useEffect(() => {
 setTimeout(() => {


Why do we wrap this in a 0 setTimeout? Is it a racing condition?

Leftover probably, removed it

abitbetterthanyesterday · 2023-01-09T22:33:58Z

tests/jest/view/plugin/search/search.test.tsx

- expect(mock).toBeCalledWith('123');
+ return new Promise<void>((resolve) => {
+ // TODO: can we fix this and remove the setTimeout?
+ setTimeout(() => {


I know with Testing library you have access to a 'waitfor' function that basically tries your assertion for X seconds.

Maybe enzyme has something similar?
Or we could rig our own?

I'm planning on migrating to the React Testing Library which could potentially help us remove these setTimeouts.

abitbetterthanyesterday · 2023-01-09T22:34:46Z

tsconfig.json

+ "declaration": true,
+ "baseUrl": "./",
+ "paths": {
+ "react": ["./node_modules/preact/compat/"],


that's interesting, what's the logic behind this?

I vaguely remember that was a short-term solution for a weird Preact/TypeScript bug. We should be able to remove this in the future.

Co-authored-by: abitbetterthanyesterday <[email protected]>

afshinm · 2023-01-10T21:40:51Z

@abitbetterthanyesterday thanks for your feedback. I have made some changes to this PR based on your feedback. Let me know if we're good to merge.

afshinm · 2023-01-10T21:41:49Z

@daniel-werner @salamaashoush That would be great if you guys can also review this PR and give me feedback ❤️

daniel-werner · 2023-01-13T15:23:12Z

@daniel-werner @salamaashoush That would be great if you guys can also review this PR and give me feedback ❤️

@afshinm I took a quick look, and looks good at first sight, however I am not familiar with the code, and the PR is quite big.

I'll let you know, and contribute if I have some problems integrating/using it in my laravel datagrid package.

afshinm changed the title ~~Functional Components~~ Grid.js v6 - Functional Components Jan 1, 2023

afshinm added bug fix Something isn't working new feature New feature or request performance breaking labels Jan 1, 2023

afshinm marked this pull request as ready for review January 1, 2023 15:20

afshinm requested a review from abitbetterthanyesterday January 1, 2023 15:48

afshinm added 21 commits January 1, 2023 15:49

feat: functional header component

28f4e2e

feat: functional footer component

386d480

chore: typescript > 4.1 config

c8949e4

chore: functional td component

e97dc15

feat: functional TH component

faa42ce

feat: functional TR component

b17c358

feat: functional Resize component

a80bb7f

feat: functional Shadow table

8c9b0cf

feat: functional HTMLElement

f00eccb

feat: functional pagination and search

e83bf08

fix: shadow table and get widths fn

62394f6

fix: typescript jsx settings

7ca6354

fix: pagination and removing sortRef

4a1e4c1

feat: addin the state management hooks + refactoring the Search plugin

0c38258

feat: refactor Sort and delete the legacy Dispatcher/Store

40d9d6b

feat: refactor Sort and delete the legacy Dispatcher/Store

91c1aa2

feat: functional RowSelection plugin

f702054

feat: global state

6df9fd8

chore: fix tests

4dde7aa

fix: functional ShadowTable + adding tableRef to state

c453341

fix: simplify the Config

32e6cf3

afshinm added 5 commits January 1, 2023 15:50

chore: remove node 12

ac577c2

chore: fix search tests

da0c99d

chore: adding node 18

9882fdf

chore: remove node 18

36f1cfc

chore: search input tests

efb4142

afshinm force-pushed the functional branch from d5dac87 to efb4142 Compare January 1, 2023 15:50

tests: selection actions tests

86d67b1

afshinm requested a review from salamaashoush January 2, 2023 10:57

abitbetterthanyesterday reviewed Jan 2, 2023

View reviewed changes

abitbetterthanyesterday self-requested a review January 2, 2023 21:45

afshinm added 3 commits January 3, 2023 09:35

fix: Store type

4194741

chore: code coverage job

516c0fc

fix: Store type

0a9f57f

abitbetterthanyesterday approved these changes Jan 9, 2023

View reviewed changes

Update src/header.ts

daeaaa4

Co-authored-by: abitbetterthanyesterday <[email protected]>

afshinm mentioned this pull request Jan 10, 2023

Server Side Search Deletes Search Text #1081

Closed

fix: remove the th setTimeout

95d8564

afshinm requested a review from daniel-werner January 10, 2023 21:41

This was referenced Jan 10, 2023

Plugins disappear after updating config. #1260

Open

forceRender() doesn't update column width according to remaining space #1247

Open

Bug on forceRender() method #1217

Closed

Just force reload #1243

Open

Resizable not working #1210

Open

afshinm merged commit 51e0818 into master Jan 16, 2023

afshinm deleted the functional branch January 16, 2023 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grid.js v6 - Functional Components #1267

Grid.js v6 - Functional Components #1267

afshinm commented Dec 25, 2022 •

edited

github-actions bot commented Dec 28, 2022 •

edited

github-actions bot commented Dec 29, 2022 •

edited

abitbetterthanyesterday commented Jan 2, 2023

abitbetterthanyesterday Jan 2, 2023

afshinm Jan 3, 2023

github-actions bot commented Jan 3, 2023 •

edited

abitbetterthanyesterday left a comment

abitbetterthanyesterday Jan 9, 2023

afshinm Jan 10, 2023

abitbetterthanyesterday Jan 9, 2023

abitbetterthanyesterday Jan 9, 2023

afshinm Jan 10, 2023

abitbetterthanyesterday Jan 9, 2023

afshinm Jan 10, 2023

abitbetterthanyesterday Jan 9, 2023

afshinm Jan 10, 2023

afshinm commented Jan 10, 2023

afshinm commented Jan 10, 2023

daniel-werner commented Jan 13, 2023

		@@ -1,16 +1,18 @@
		import { h } from 'preact';
		import { h, JSX } from 'preact';

Grid.js v6 - Functional Components #1267

Grid.js v6 - Functional Components #1267

Conversation

afshinm commented Dec 25, 2022 • edited

Grid.js v6 🚀

Context

Why?

Feedback

github-actions bot commented Dec 28, 2022 • edited

github-actions bot commented Dec 29, 2022 • edited

jest coverage report 🧪

abitbetterthanyesterday commented Jan 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 3, 2023 • edited

Coverage report

Test suite run success

abitbetterthanyesterday left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afshinm commented Jan 10, 2023

afshinm commented Jan 10, 2023

daniel-werner commented Jan 13, 2023

afshinm commented Dec 25, 2022 •

edited

github-actions bot commented Dec 28, 2022 •

edited

github-actions bot commented Dec 29, 2022 •

edited

github-actions bot commented Jan 3, 2023 •

edited