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

Reorganize shared utils #1539

Merged
merged 2 commits into from
Jan 10, 2023
Merged

Reorganize shared utils #1539

merged 2 commits into from
Jan 10, 2023

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Jan 10, 2023

Create a useNonNullableContext function to replace createProvidedContext down the line. The problem with createProvidedContext is that it makes it impossible for the react eslint rules to detect stable/unstable context values. In this PR I'm replacing some instances of createProvidedContext with this new hook and fixing the associate eslint issues

I'm also reordering a bit the utility imports from @mui/toolpad-core in anticipation of creating a utils package that we can reuse. @mui/toolpad-core is intended to share logic between the components and the runtime, we should aim at keeping it lean, utilities meant for broader use can go in new package. For this package we will encourage subpath imports to maximize treeshakeability e.g.

import { interleaveJsx } from '@mui/toolpad-utils/react'

I'm not making that switch in this PR, but as a first step, I'm just disallowing importing utilities from @mui/toolpad-core.

@oliviertassinari oliviertassinari requested a deployment to reorg-shared-utils - toolpad-db PR #1539 January 10, 2023 10:34 — with Render Abandoned
@Janpot Janpot added the core Infrastructure work going on behind the scenes label Jan 10, 2023
if (!mod.error) {
const cacheId = `// ${id}\n${mod.code}`;
const fromCache = cache.get(cacheId);
const loadedModules = React.useMemo(() => {
Copy link
Member Author

@Janpot Janpot Jan 10, 2023

Choose a reason for hiding this comment

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

@Janpot Janpot marked this pull request as ready for review January 10, 2023 14:09
Copy link
Member

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

I'm also reordering a bit the utility imports from @mui/toolpad-core in anticipation of creating a utils package that we can reuse. @mui/toolpad-core is intended to share logic between the components and the runtime, we should aim at keeping it lean, utilities meant for broader use can go in new package. For this package we will encourage subpath imports to maximize treeshakeability e.g.

Sounds great to keep the bundle size low!

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 10, 2023
Signed-off-by: Jan Potoms <2109932+Janpot@users.noreply.github.com>
@oliviertassinari oliviertassinari temporarily deployed to reorg-shared-utils - toolpad PR #1539 January 10, 2023 16:31 — with Render Destroyed
@Janpot Janpot enabled auto-merge (squash) January 10, 2023 16:31
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 10, 2023
@Janpot Janpot merged commit 295aafc into master Jan 10, 2023
@Janpot Janpot deleted the reorg-shared-utils branch January 10, 2023 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants