Replies: 6 comments 5 replies
-
|
If I remember correctly the reason why |
Beta Was this translation helpful? Give feedback.
-
|
i made a couple of WIP branches here in case anyone gets to finish them off: switching from execa to tinyexec: this one doesn't build yet since jest seems to behind the times re ESM. so it can't load tinyexec it or any other ESM package it seems removing fs-extra from create-react-router: removing recursive-readdir from create-react-router: and some other findings:
|
Beta Was this translation helpful? Give feedback.
-
Beta Was this translation helpful? Give feedback.
-
|
I'll add on eslint, I was curious to try it out (port CRA config for legacy eslint to flat config) and got this: https://github.com/outslept/eslint-config-react-app-x This works locally with |
Beta Was this translation helpful? Give feedback.
-
also Note Consider leveraging pnpm's hoisting mechanism to eliminate duplicate dependencies between root and workspace packages. Dependencies declared in the root |
Beta Was this translation helpful? Give feedback.
-
|
And in addition to the note above I ran knip preconfigured like this (feel free to tweak the settings and try to run it as well, I'll be happy if you can find something else): Knip Configurationimport { readFileSync, existsSync, readdirSync } from 'node:fs';
import { join } from 'node:path';
function getWorkspaces() {
const workspaces = ['integration'];
try {
workspaces.push(...readdirSync('integration/helpers', { withFileTypes: true })
.filter(d => d.isDirectory())
.map(d => `integration/helpers/${d.name}`));
} catch {}
try {
workspaces.push(...readdirSync('packages', { withFileTypes: true })
.filter(d => d.isDirectory())
.map(d => `packages/${d.name}`));
} catch {}
try {
workspaces.push(...readdirSync('playground', { withFileTypes: true })
.filter(d => d.isDirectory())
.map(d => `playground/${d.name}`));
} catch {}
return workspaces;
}
function getTsupEntry(path) {
const cfg = join(path, 'tsup.config.ts');
if (!existsSync(cfg)) return [];
const match = readFileSync(cfg).toString().match(/const entry = \[(.*?)\];/s);
if (!match) return [];
return match[1]
.split(',')
.map(e => e.trim().replace(/['"]/g, ''))
.filter(Boolean)
.filter(e => !['index.ts', 'main.ts', 'cli.ts'].includes(e));
}
function getProjectPatterns(path) {
const base = ['**/*.{ts,tsx,js,jsx}', '!node_modules/**', '!dist/**', '!build/**'];
if (path === 'packages/react-router-dev') {
return [...base, '!config/defaults/**', '!__tests__/utils/**', '!__tests__/fixtures/**'];
}
if (path === 'packages/react-router') {
return [...base, '!node-main*.js', '!lib/dom/node-main.js', '!lib/server-runtime/.eslintrc.js', '!__tests__/utils/**'];
}
if (path === 'packages/create-react-router') {
return [...base, '!__tests__/msw-register.ts', '!__tests__/fixtures/**'];
}
return base;
}
const config = {
includeEntryExports: true,
ignoreUnresolved: [/^\.\/\+types\//,],
workspaces: {
'.': {
entry: ['scripts/*.{js,ts,mjs}'],
project: ['scripts/**/*', 'jest/**/*', '*.{js,ts,mjs}'],
ignore: ['scripts/utils.js']
}
}
};
for (const workspace of getWorkspaces()) {
const entries = getTsupEntry(workspace);
config.workspaces[workspace] = {
entry: entries.length ? entries : undefined,
project: getProjectPatterns(workspace),
includeEntryExports: true
};
}
export default config;I manually went through all entries and marked what can be removed/deleted with some notes at the end of the details. Unused files (13)Most of these can be ignored as they are config files, etc. The Unused exports (49)I'm not touching the types to avoid messing with what you want to expose in the public API. |
Beta Was this translation helpful? Give feedback.
-
|
i wonder if some of those entry points can be inferred since, iirc, knip checks what |
Beta Was this translation helpful? Give feedback.
-
|
Really nice finding! I've updated the config to reflect this |
Beta Was this translation helpful? Give feedback.
-
|
With these changes to |
Beta Was this translation helpful? Give feedback.
-
|
People may still want to use the server inside an existing node server. So if they use express, polka or some other server - they can support react router by middleware. With the suggested changes it's basically a generic middleware Maybe the node package could export that, though 🤔 |
Beta Was this translation helpful? Give feedback.
-
|
wip branch main...outslept:react-router:refactor/knip-cleanup I also updated the knip config from above again, there are a couple more issues that need to be resolved before I can add this as a separate script to the repository |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Remix is one of the few orgs we haven't interacted with much in the e18e community. I think it'd be nice to increase the collaboration here if you're interested. You can read about what the community is doing on e18e.dev.
this is just my list of findings. if any seem like something we should do here, please comment as I'm sure there will be community members willing to tackle them in PRs.
similarly, if any don't make sense, I'm happy to remove them from the list or note that people shouldn't contribute the change.
nobody should pick these up yet. I'm just looking for confirmation from maintainers that some are worth doing, then someone can pick them up.
repo-wide tasks
eslint.config.js)rimraftopremove(orfs.rmif programmatic usage)create-react-router
General changes
enginesconstraint to 20.x? since the other monorepo packages seem to already have done thisDependency changes
fs-extrawith nativefs.cp(...)fs-extra#13736execawith tinyexecchalkwith ansis (not picocolors because we use hex colours here)tar-fswithtar(latesttar-fsis much larger, but we haven't upgraded to it here yet. both seem to perform well enough for the usage here)recursive-readdirwith nativereaddir(path, {recursive: true})recursive-readdir#13735Refactors
react-router-dev
Dependency changes
fs-extrawith native FSfs-extra#13736lodashisEqualcan probably be replaced withdeep-eql(from chai) ordequal(smaller)cloneDeepwith klona, rfdc or nanocloneomitis only used once, so maybe we can just use spread or remove the keys manuallypickcould possibly be inlined as a util, or justfiltertheObject.entrieskebabCasewithsculereact-router-fs-routes
Dependency changes
minimatchwithzeptomatch(low priority change of course)Refactors
replaceAllinstead of asplit/joinin normalizeSlashes (should be faster, but if not, areplacewith a global regex should be)replaceAllfor slash normalisation #13738ignoreFileRegexafter the various error checks to avoid it being wasted/garbage collectedreact-router-serve
Dependency changes
react-router-express
Refactors
existing work
cc @MichaelDeBoey since you were already doing some of this
Beta Was this translation helpful? Give feedback.
All reactions