Skip to content

Commit

Permalink
CLI responsiveness (redwoodjs#6028)
Browse files Browse the repository at this point in the history
* Move build handler.

* Move dev handler.

* move console handler.

* Do not import structure.

* Use local getPaths.

* Attempt to directly import methods.

* Make prerender async.

* Rename file, add "sides."

* Use renamed file.

* Use new-sides.

* Remove unused imports.

* Async exec handler.

* Async prisma handler.

* Async type check.

* Make build use sides.

* Make serve use getPaths.

* Return of the middleware.

* revert internal import change to see if tests pass

* fix type-check by fixing sides import

* Revert "revert internal import change to see if tests pass"

This reverts commit 2db74ad.

* update mocking for cli tests accordingly

* fix import to getConigPath

* Add lint rule to prevent direct import of redwoodjs/internal inside framework

* Fix all lint errors and tests

* Update eslint comments

* Import prerender tasks from the correct place

Co-authored-by: Dominic Saadi <[email protected]>
Co-authored-by: Daniel Choudhury <[email protected]>
  • Loading branch information
3 people authored Jul 28, 2022
1 parent d67e937 commit f262bab
Show file tree
Hide file tree
Showing 71 changed files with 1,334 additions and 1,136 deletions.
58 changes: 58 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,38 @@ module.exports = {
window: 'off', // Developers should use `global` instead of window. Since window is undefined in NodeJS.
},
},
// Prevent @redwoodjs/internal imports in runtime (web+api) packages
{
files: [
'packages/auth/src/**',
'packages/forms/src/**',
'packages/prerender/src/browserUtils/**',
'packages/router/src/**',
'packages/web/src/**',
'packages/api/src/**',
'packages/graphql-server/src/**',
'packages/record/src/**',
],
rules: {
'no-restricted-imports': [
'error',
{
patterns: [
{
group: ['@redwoodjs/internal', '@redwoodjs/internal/*'],
message:
'Do not import "@redwoodjs/internal" or subpackages in runtime modules, because it leads to MASSIVE bundle sizes',
},
{
group: ['@redwoodjs/structure', '@redwoodjs/structure/*'],
message:
'Do not import "@redwoodjs/structure" or subpackages in runtime modules, because it leads to MASSIVE bundle sizes',
},
],
},
],
},
},
// Entry.js rules
{
files: ['packages/web/src/entry/index.js'],
Expand Down Expand Up @@ -109,5 +141,31 @@ module.exports = {
node: true,
},
},
// Prevent bad imports in Node packages - cli and api packages
{
files: [
'packages/api/src/**',
'packages/api-server/src/**',
'packages/cli/src/**',
'packages/internal/src/**',
'packages/prerender/src/**',
'packages/structure/src/**',
'packages/testing/src/**',
'packages/testing/config/**',
'packages/eslint-config/*.js',
'packages/record/src/**',
'packages/telemetry/src/**',
],
rules: {
'no-restricted-imports': [
'error',
{
name: '@redwoodjs/internal',
message:
'To prevent bloat in CLI, do not import "@redwoodjs/internal" directly. Instead import like @redwoodjs/internal/dist/<file>, or await import',
},
],
},
},
],
}
4 changes: 2 additions & 2 deletions packages/api-server/src/__tests__/withWebServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ const FIXTURE_PATH = path.resolve(

// Mock the dist folder from fixtures,
// because its gitignored
jest.mock('@redwoodjs/internal', () => {
jest.mock('@redwoodjs/internal/dist/files', () => {
return {
...jest.requireActual('@redwoodjs/internal'),
...jest.requireActual('@redwoodjs/internal/dist/files'),
findPrerenderedHtml: () => {
return ['about.html', 'mocked.html', 'posts/new.html', 'index.html']
},
Expand Down
3 changes: 2 additions & 1 deletion packages/api-server/src/cliHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import path from 'path'
import c from 'ansi-colors'
import { FastifyServerOptions } from 'fastify'

import { getConfig, getPaths } from '@redwoodjs/internal'
import { getConfig } from '@redwoodjs/internal/dist/config'
import { getPaths } from '@redwoodjs/internal/dist/paths'

import createApp from './app'
import withApiProxy from './plugins/withApiProxy'
Expand Down
2 changes: 1 addition & 1 deletion packages/api-server/src/plugins/withFunctions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import fastifyRawBody from 'fastify-raw-body'
import escape from 'lodash.escape'

import { findApiDistFunctions } from '@redwoodjs/internal'
import { findApiDistFunctions } from '@redwoodjs/internal/dist/files'

import { requestHandler } from '../requestHandlers/awsLambdaFastify'

Expand Down
3 changes: 2 additions & 1 deletion packages/api-server/src/plugins/withWebServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import path from 'path'
import fastifyStatic from '@fastify/static'
import { FastifyInstance, FastifyReply } from 'fastify'

import { findPrerenderedHtml, getPaths } from '@redwoodjs/internal'
import { findPrerenderedHtml } from '@redwoodjs/internal/dist/files'
import { getPaths } from '@redwoodjs/internal/dist/paths'

export const getFallbackIndexPath = () => {
const prerenderIndexPath = path.join(getPaths().web.dist, '/200.html')
Expand Down
11 changes: 4 additions & 7 deletions packages/api-server/src/watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,10 @@ import { debounce } from 'lodash'
import { hideBin } from 'yargs/helpers'
import yargs from 'yargs/yargs'

import {
getPaths,
buildApi,
getConfig,
ensurePosixPath,
loadAndValidateSdls,
} from '@redwoodjs/internal'
import { buildApi } from '@redwoodjs/internal/dist/build/api'
import { getConfig } from '@redwoodjs/internal/dist/config'
import { getPaths, ensurePosixPath } from '@redwoodjs/internal/dist/paths'
import { loadAndValidateSdls } from '@redwoodjs/internal/dist/validateSchema'

const argv = yargs(hideBin(process.argv))
.option('debug-port', {
Expand Down
13 changes: 9 additions & 4 deletions packages/cli/src/commands/__tests__/build.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
jest.mock('@redwoodjs/internal', () => {
jest.mock('@redwoodjs/internal/dist/paths', () => {
return {
getPaths: () => {
return {
Expand All @@ -10,6 +10,11 @@ jest.mock('@redwoodjs/internal', () => {
},
}
},
}
})

jest.mock('@redwoodjs/internal/dist/config', () => {
return {
getConfig: () => {},
}
})
Expand All @@ -25,8 +30,8 @@ import { handler } from '../build'

afterEach(() => jest.clearAllMocks())

test('the build tasks are in the correct sequence', () => {
handler({})
test('the build tasks are in the correct sequence', async () => {
await handler({})
expect(Listr.mock.calls[0][0].map((x) => x.title)).toMatchInlineSnapshot(`
Array [
"Generating Prisma Client...",
Expand All @@ -43,7 +48,7 @@ jest.mock('@redwoodjs/prerender/detection', () => {
})

test('Should run prerender for web', async () => {
handler({ side: ['web'], prerender: true })
await handler({ side: ['web'], prerender: true })
expect(Listr.mock.calls[0][0].map((x) => x.title)).toMatchInlineSnapshot(`
Array [
"Cleaning Web...",
Expand Down
22 changes: 17 additions & 5 deletions packages/cli/src/commands/__tests__/dev.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import '../../lib/mockTelemetry'

jest.mock('concurrently', () => ({
__esModule: true, // this property makes it work
default: jest.fn().mockReturnValue({
Expand All @@ -16,8 +18,21 @@ jest.mock('fs', () => {
}
})

jest.mock('@redwoodjs/internal', () => {
jest.mock('@redwoodjs/internal/dist/config', () => {
return {
getConfig: jest.fn(),
}
})

jest.mock('@redwoodjs/internal/dist/dev', () => {
return {
shutdownPort: jest.fn(),
}
})

jest.mock('@redwoodjs/internal/dist/paths', () => {
return {
getConfigPath: () => '/mocked/project/redwood.toml',
getPaths: () => {
return {
api: {
Expand All @@ -31,9 +46,6 @@ jest.mock('@redwoodjs/internal', () => {
},
}
},
getConfig: jest.fn(),
getConfigPath: () => '/mocked/project/redwood.toml',
shutdownPort: jest.fn(),
}
})

Expand All @@ -46,7 +58,7 @@ jest.mock('../../lib/generatePrismaClient', () => {
import concurrently from 'concurrently'
import { find } from 'lodash'

import { getConfig } from '@redwoodjs/internal'
import { getConfig } from '@redwoodjs/internal/dist/config'

import { generatePrismaClient } from '../../lib/generatePrismaClient'
import { handler } from '../dev'
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/__tests__/prisma.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
jest.mock('@redwoodjs/internal', () => {
jest.mock('@redwoodjs/internal/dist/paths', () => {
return {
getPaths: () => {
return {
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/commands/__tests__/serve.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
global.__dirname = __dirname

// We mock these to skip the check for web/dist and api/dist
jest.mock('@redwoodjs/internal', () => {
jest.mock('@redwoodjs/internal/dist/paths', () => {
return {
getPaths: () => {
return {
Expand All @@ -13,6 +13,11 @@ jest.mock('@redwoodjs/internal', () => {
},
}
},
}
})

jest.mock('@redwoodjs/internal/dist/config', () => {
return {
getConfig: () => {
return {
api: {},
Expand Down
Loading

0 comments on commit f262bab

Please sign in to comment.