diff --git a/react_on_rails_pro/packages/node-renderer/src/worker/checkProtocolVersionHandler.ts b/react_on_rails_pro/packages/node-renderer/src/worker/checkProtocolVersionHandler.ts index b0eab7f5db..0d3a382822 100644 --- a/react_on_rails_pro/packages/node-renderer/src/worker/checkProtocolVersionHandler.ts +++ b/react_on_rails_pro/packages/node-renderer/src/worker/checkProtocolVersionHandler.ts @@ -34,7 +34,7 @@ function normalizeVersion(version: string): string { return normalized; } -interface RequestBody { +export interface RequestBody { protocolVersion?: string; gemVersion?: string; railsEnv?: string; diff --git a/react_on_rails_pro/packages/node-renderer/src/worker/requestPrechecks.ts b/react_on_rails_pro/packages/node-renderer/src/worker/requestPrechecks.ts index 737df00fc8..b17f074e31 100644 --- a/react_on_rails_pro/packages/node-renderer/src/worker/requestPrechecks.ts +++ b/react_on_rails_pro/packages/node-renderer/src/worker/requestPrechecks.ts @@ -3,10 +3,10 @@ * @module worker/requestPrechecks */ import type { ResponseResult } from '../shared/utils'; -import { checkProtocolVersion, type ProtocolVersionBody } from './checkProtocolVersionHandler'; +import { checkProtocolVersion, type RequestBody } from './checkProtocolVersionHandler'; import { authenticate, type AuthBody } from './authHandler'; -export interface RequestPrechecksBody extends ProtocolVersionBody, AuthBody { +export interface RequestPrechecksBody extends RequestBody, AuthBody { [key: string]: unknown; } diff --git a/react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle-incremental.js b/react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle-incremental.js new file mode 100644 index 0000000000..bc41ebb738 --- /dev/null +++ b/react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle-incremental.js @@ -0,0 +1,41 @@ +const { PassThrough } = require('stream'); + +global.ReactOnRails = { + dummy: { html: 'Dummy Object' }, + + // Get or create stream + getStreamValues: function () { + if (!sharedExecutionContext.has('stream')) { + const stream = new PassThrough(); + sharedExecutionContext.set('stream', { stream }); + } + return sharedExecutionContext.get('stream').stream; + }, + + // Add value to stream + addStreamValue: function (value) { + if (!sharedExecutionContext.has('stream')) { + // Create the stream first if it doesn't exist + ReactOnRails.getStreamValues(); + } + const { stream } = sharedExecutionContext.get('stream'); + stream.write(value); + return value; + }, + + endStream: function () { + if (sharedExecutionContext.has('stream')) { + const { stream } = sharedExecutionContext.get('stream'); + stream.end(); + } + }, + + // Clear all stream values + clearStreamValues: function () { + if (sharedExecutionContext.has('stream')) { + const { stream } = sharedExecutionContext.get('stream'); + stream.destroy(); + sharedExecutionContext.delete('stream'); + } + }, +}; diff --git a/react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle.js b/react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle.js index b75ede3f5c..4ed2eac53f 100644 --- a/react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle.js +++ b/react_on_rails_pro/packages/node-renderer/tests/fixtures/bundle.js @@ -1,57 +1,3 @@ -const { PassThrough } = require('stream'); - global.ReactOnRails = { dummy: { html: 'Dummy Object' }, - - // Get or create async value promise - getAsyncValue: function() { - debugger; - if (!sharedExecutionContext.has('asyncPromise')) { - const promiseData = {}; - const promise = new Promise((resolve, reject) => { - promiseData.resolve = resolve; - promiseData.reject = reject; - }); - promiseData.promise = promise; - sharedExecutionContext.set('asyncPromise', promiseData); - } - return sharedExecutionContext.get('asyncPromise').promise; - }, - - // Resolve the async value promise - setAsyncValue: function(value) { - debugger; - if (!sharedExecutionContext.has('asyncPromise')) { - ReactOnRails.getAsyncValue(); - } - const promiseData = sharedExecutionContext.get('asyncPromise'); - promiseData.resolve(value); - }, - - // Get or create stream - getStreamValues: function() { - if (!sharedExecutionContext.has('stream')) { - const stream = new PassThrough(); - sharedExecutionContext.set('stream', { stream }); - } - return sharedExecutionContext.get('stream').stream; - }, - - // Add value to stream - addStreamValue: function(value) { - if (!sharedExecutionContext.has('stream')) { - // Create the stream first if it doesn't exist - ReactOnRails.getStreamValues(); - } - const { stream } = sharedExecutionContext.get('stream'); - stream.write(value); - return value; - }, - - endStream: function() { - if (sharedExecutionContext.has('stream')) { - const { stream } = sharedExecutionContext.get('stream'); - stream.end(); - } - }, }; diff --git a/react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle-incremental.js b/react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle-incremental.js new file mode 100644 index 0000000000..7a8637c4c8 --- /dev/null +++ b/react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle-incremental.js @@ -0,0 +1,40 @@ +const { PassThrough } = require('stream'); + +global.ReactOnRails = { + dummy: { html: 'Dummy Object from secondary bundle' }, + + // Get or create stream + getStreamValues: function () { + if (!sharedExecutionContext.has('secondaryStream')) { + const stream = new PassThrough(); + sharedExecutionContext.set('secondaryStream', { stream }); + } + return sharedExecutionContext.get('secondaryStream').stream; + }, + + // Add value to stream + addStreamValue: function (value) { + if (!sharedExecutionContext.has('secondaryStream')) { + // Create the stream first if it doesn't exist + ReactOnRails.getStreamValues(); + } + const { stream } = sharedExecutionContext.get('secondaryStream'); + stream.write(value); + }, + + endStream: function () { + if (sharedExecutionContext.has('secondaryStream')) { + const { stream } = sharedExecutionContext.get('secondaryStream'); + stream.end(); + } + }, + + // Clear all stream values + clearStreamValues: function () { + if (sharedExecutionContext.has('secondaryStream')) { + const { stream } = sharedExecutionContext.get('secondaryStream'); + stream.destroy(); + sharedExecutionContext.delete('secondaryStream'); + } + }, +}; diff --git a/react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle.js b/react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle.js index cde44a80f7..d901dd0526 100644 --- a/react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle.js +++ b/react_on_rails_pro/packages/node-renderer/tests/fixtures/secondary-bundle.js @@ -1,53 +1,3 @@ global.ReactOnRails = { dummy: { html: 'Dummy Object from secondary bundle' }, - - - // Get or create async value promise - getAsyncValue: function() { - if (!sharedExecutionContext.has('secondaryAsyncPromise')) { - const promiseData = {}; - const promise = new Promise((resolve, reject) => { - promiseData.resolve = resolve; - promiseData.reject = reject; - }); - promiseData.promise = promise; - sharedExecutionContext.set('secondaryAsyncPromise', promiseData); - } - return sharedExecutionContext.get('secondaryAsyncPromise').promise; - }, - - // Resolve the async value promise - setAsyncValue: function(value) { - if (!sharedExecutionContext.has('secondaryAsyncPromise')) { - ReactOnRails.getAsyncValue(); - } - const promiseData = sharedExecutionContext.get('secondaryAsyncPromise'); - promiseData.resolve(value); - }, - - // Get or create stream - getStreamValues: function() { - if (!sharedExecutionContext.has('secondaryStream')) { - const stream = new PassThrough(); - sharedExecutionContext.set('secondaryStream', { stream }); - } - return sharedExecutionContext.get('secondaryStream').stream; - }, - - // Add value to stream - addStreamValue: function(value) { - if (!sharedExecutionContext.has('secondaryStream')) { - // Create the stream first if it doesn't exist - ReactOnRails.getStreamValues(); - } - const { stream } = sharedExecutionContext.get('secondaryStream'); - stream.write(value); - }, - - endStream: function() { - if (sharedExecutionContext.has('secondaryStream')) { - const { stream } = sharedExecutionContext.get('secondaryStream'); - stream.end(); - } - }, }; diff --git a/react_on_rails_pro/packages/node-renderer/tests/handleRenderRequest.test.ts b/react_on_rails_pro/packages/node-renderer/tests/handleRenderRequest.test.ts index 2557fa78d8..68c5c8230b 100644 --- a/react_on_rails_pro/packages/node-renderer/tests/handleRenderRequest.test.ts +++ b/react_on_rails_pro/packages/node-renderer/tests/handleRenderRequest.test.ts @@ -78,7 +78,7 @@ describe(testName, () => { ], }); - expect(result).toEqual(renderResult); + expect(result.response).toEqual(renderResult); expect( hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)), ).toBeTruthy(); @@ -92,7 +92,7 @@ describe(testName, () => { bundleTimestamp: BUNDLE_TIMESTAMP, }); - expect(result).toEqual({ + expect(result.response).toEqual({ status: 410, headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' }, data: 'No bundle uploaded', @@ -108,7 +108,7 @@ describe(testName, () => { bundleTimestamp: BUNDLE_TIMESTAMP, }); - expect(result).toEqual(renderResult); + expect(result.response).toEqual(renderResult); }); test('If lockfile exists, and is stale', async () => { @@ -133,7 +133,7 @@ describe(testName, () => { ], }); - expect(result).toEqual(renderResult); + expect(result.response).toEqual(renderResult); expect( hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)), ).toBeTruthy(); @@ -165,7 +165,7 @@ describe(testName, () => { ], }); - expect(result).toEqual(renderResult); + expect(result.response).toEqual(renderResult); expect( hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)), ).toBeTruthy(); @@ -199,7 +199,7 @@ describe(testName, () => { ], }); - expect(result).toEqual(renderResult); + expect(result.response).toEqual(renderResult); // only the primary bundle should be in the VM context // The secondary bundle will be processed only if the rendering request requests it expect( @@ -254,7 +254,7 @@ describe(testName, () => { assetsToCopy: additionalAssets, }); - expect(result).toEqual(renderResult); + expect(result.response).toEqual(renderResult); // Only the primary bundle should be in the VM context // The secondary bundle will be processed only if the rendering request requests it @@ -310,7 +310,7 @@ describe(testName, () => { dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP], }); - expect(result).toEqual({ + expect(result.response).toEqual({ status: 410, headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' }, data: 'No bundle uploaded', @@ -328,7 +328,7 @@ describe(testName, () => { dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP], }); - expect(result).toEqual(renderResult); + expect(result.response).toEqual(renderResult); }); test('rendering request can call runOnOtherBundle', async () => { @@ -348,7 +348,7 @@ describe(testName, () => { dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP], }); - expect(result).toEqual(renderResultFromBothBundles); + expect(result.response).toEqual(renderResultFromBothBundles); // Both bundles should be in the VM context expect( hasVMContextForBundle(path.resolve(__dirname, `./tmp/${testName}/1495063024898/1495063024898.js`)), @@ -370,7 +370,7 @@ describe(testName, () => { bundleTimestamp: BUNDLE_TIMESTAMP, }); - expect(result).toEqual({ + expect(result.response).toEqual({ status: 200, headers: { 'Cache-Control': 'public, max-age=31536000' }, data: renderingRequest, @@ -402,7 +402,7 @@ describe(testName, () => { bundleTimestamp: BUNDLE_TIMESTAMP, }); - expect(result).toEqual({ + expect(result.response).toEqual({ status: 200, headers: { 'Cache-Control': 'public, max-age=31536000' }, data: JSON.stringify('undefined'), @@ -420,7 +420,7 @@ describe(testName, () => { dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP], }); - expect(result).toEqual({ + expect(result.response).toEqual({ status: 410, headers: { 'Cache-Control': 'no-cache, no-store, max-age=0, must-revalidate' }, data: 'No bundle uploaded', diff --git a/react_on_rails_pro/packages/node-renderer/tests/helper.ts b/react_on_rails_pro/packages/node-renderer/tests/helper.ts index 3fbfc5e12a..86cfdd033f 100644 --- a/react_on_rails_pro/packages/node-renderer/tests/helper.ts +++ b/react_on_rails_pro/packages/node-renderer/tests/helper.ts @@ -27,6 +27,14 @@ export function getFixtureSecondaryBundle() { return path.resolve(__dirname, './fixtures/secondary-bundle.js'); } +export function getFixtureIncrementalBundle() { + return path.resolve(__dirname, './fixtures/bundle-incremental.js'); +} + +export function getFixtureIncrementalSecondaryBundle() { + return path.resolve(__dirname, './fixtures/secondary-bundle-incremental.js'); +} + export function getFixtureAsset() { return path.resolve(__dirname, `./fixtures/${ASSET_UPLOAD_FILE}`); } @@ -58,24 +66,36 @@ export function vmSecondaryBundlePath(testName: string) { } export async function createVmBundle(testName: string) { + // Build config with module support before creating VM bundle + await safeCopyFileAsync(getFixtureBundle(), vmBundlePath(testName)); + await buildExecutionContext([vmBundlePath(testName)], /* buildVmsIfNeeded */ true); +} + +export async function createSecondaryVmBundle(testName: string) { + // Build config with module support before creating VM bundle + await safeCopyFileAsync(getFixtureSecondaryBundle(), vmSecondaryBundlePath(testName)); + await buildExecutionContext([vmSecondaryBundlePath(testName)], /* buildVmsIfNeeded */ true); +} + +export async function createIncrementalVmBundle(testName: string) { // Build config with module support before creating VM bundle buildConfig({ - bundlePath: bundlePath(testName), + serverBundleCachePath: serverBundleCachePath(testName), supportModules: true, stubTimers: false, }); - await safeCopyFileAsync(getFixtureBundle(), vmBundlePath(testName)); + await safeCopyFileAsync(getFixtureIncrementalBundle(), vmBundlePath(testName)); await buildExecutionContext([vmBundlePath(testName)], /* buildVmsIfNeeded */ true); } -export async function createSecondaryVmBundle(testName: string) { +export async function createIncrementalSecondaryVmBundle(testName: string) { // Build config with module support before creating VM bundle buildConfig({ - bundlePath: bundlePath(testName), + serverBundleCachePath: serverBundleCachePath(testName), supportModules: true, stubTimers: false, }); - await safeCopyFileAsync(getFixtureSecondaryBundle(), vmSecondaryBundlePath(testName)); + await safeCopyFileAsync(getFixtureIncrementalSecondaryBundle(), vmSecondaryBundlePath(testName)); await buildExecutionContext([vmSecondaryBundlePath(testName)], /* buildVmsIfNeeded */ true); } @@ -140,10 +160,12 @@ export async function createAsset(testName: string, bundleTimestamp: string) { ]); } -export async function resetForTest(testName: string) { +export async function resetForTest(testName: string, resetConfigs = true) { await fsExtra.emptyDir(serverBundleCachePath(testName)); resetVM(); - setConfig(testName); + if (resetConfigs) { + setConfig(testName); + } } export function readRenderingRequest(projectName: string, commit: string, requestDumpFileName: string) { @@ -201,5 +223,3 @@ export const waitFor = async ( const defaultMessage = `Expect condition not met within ${timeout}ms`; throw new Error(message || defaultMessage + (lastError ? `\nLast error: ${lastError.message}` : '')); }; - -setConfig('helper'); diff --git a/react_on_rails_pro/packages/node-renderer/tests/incrementalRender.test.ts b/react_on_rails_pro/packages/node-renderer/tests/incrementalRender.test.ts index 325cb9f93c..e0a79a895e 100644 --- a/react_on_rails_pro/packages/node-renderer/tests/incrementalRender.test.ts +++ b/react_on_rails_pro/packages/node-renderer/tests/incrementalRender.test.ts @@ -6,10 +6,12 @@ import packageJson from '../src/shared/packageJson'; import * as incremental from '../src/worker/handleIncrementalRenderRequest'; import { createVmBundle, - createSecondaryVmBundle, + createIncrementalVmBundle, + createIncrementalSecondaryVmBundle, BUNDLE_TIMESTAMP, SECONDARY_BUNDLE_TIMESTAMP, waitFor, + resetForTest, } from './helper'; import type { ResponseResult } from '../src/shared/utils'; @@ -236,6 +238,10 @@ describe('incremental render NDJSON endpoint', () => { return { promise, receivedChunks }; }; + beforeEach(async () => { + await resetForTest(TEST_NAME, false); + }); + afterEach(() => { jest.restoreAllMocks(); }); @@ -674,8 +680,8 @@ describe('incremental render NDJSON endpoint', () => { }); describe('incremental render update chunk functionality', () => { - test.only('basic incremental update - initial request gets value, update chunks set value', async () => { - await createVmBundle(TEST_NAME); + test('basic incremental update - initial request gets value, update chunks set value', async () => { + await createIncrementalVmBundle(TEST_NAME); const SERVER_BUNDLE_TIMESTAMP = String(BUNDLE_TIMESTAMP); // Create the HTTP request @@ -709,62 +715,8 @@ describe('incremental render NDJSON endpoint', () => { expect(response.data).toBe('first update'); // Should resolve with the first setAsyncValue call }); - test('incremental updates work with multiple bundles using runOnOtherBundle', async () => { - await createVmBundle(TEST_NAME); - await createSecondaryVmBundle(TEST_NAME); - const SERVER_BUNDLE_TIMESTAMP = String(BUNDLE_TIMESTAMP); - const SECONDARY_BUNDLE_TIMESTAMP_STR = String(SECONDARY_BUNDLE_TIMESTAMP); - - // Create the HTTP request - const req = createHttpRequest(SERVER_BUNDLE_TIMESTAMP); - - // Set up response handling - const responsePromise = setupResponseHandler(req, true); - - // Send the initial object that gets values from both bundles - const initialObject = { - ...createInitialObject(SERVER_BUNDLE_TIMESTAMP), - renderingRequest: ` - runOnOtherBundle(${SECONDARY_BUNDLE_TIMESTAMP}, 'ReactOnRails.getAsyncValue()').then((secondaryValue) => ({ - mainBundleValue: ReactOnRails.getAsyncValue(), - secondaryBundleValue: JSON.parse(secondaryValue), - })); - `, - dependencyBundleTimestamps: [SECONDARY_BUNDLE_TIMESTAMP_STR], - }; - req.write(`${JSON.stringify(initialObject)}\n`); - - // Send update chunks to both bundles - const updateMainBundle = { - bundleTimestamp: SERVER_BUNDLE_TIMESTAMP, - updateChunk: 'ReactOnRails.setAsyncValue("main bundle updated")', - }; - req.write(`${JSON.stringify(updateMainBundle)}\n`); - - const updateSecondaryBundle = { - bundleTimestamp: SECONDARY_BUNDLE_TIMESTAMP_STR, - updateChunk: 'ReactOnRails.setAsyncValue("secondary bundle updated")', - }; - req.write(`${JSON.stringify(updateSecondaryBundle)}\n`); - - // End the request - req.end(); - - // Wait for the response - const response = await responsePromise; - - // Verify the response - expect(response.statusCode).toBe(200); - const responseData = JSON.parse(response.data || '{}') as { - mainBundleValue: unknown; - secondaryBundleValue: unknown; - }; - expect(responseData.mainBundleValue).toBe('main bundle updated'); - expect(responseData.secondaryBundleValue).toBe('secondary bundle updated'); - }); - test('streaming functionality with incremental updates', async () => { - await createVmBundle(TEST_NAME); + await createIncrementalVmBundle(TEST_NAME); const SERVER_BUNDLE_TIMESTAMP = String(BUNDLE_TIMESTAMP); // Create the HTTP request @@ -802,99 +754,12 @@ describe('incremental render NDJSON endpoint', () => { req.write(`${JSON.stringify(updateChunk)}\n`); } - // No need to get stream values again since we're already streaming - - // End the request - req.end(); - - // Wait for the response - const response = await responsePromise; - - // Verify the response - expect(response.statusCode).toBe(200); - // Since we're returning a stream, the response should indicate streaming - expect(streamedData.length).toBeGreaterThan(0); - }); - - test('error handling in incremental render updates', async () => { - await createVmBundle(TEST_NAME); - const SERVER_BUNDLE_TIMESTAMP = String(BUNDLE_TIMESTAMP); - - // Create the HTTP request - const req = createHttpRequest(SERVER_BUNDLE_TIMESTAMP); - - // Set up response handling - const responsePromise = setupResponseHandler(req, true); - - // Send the initial object - const initialObject = { - ...createInitialObject(SERVER_BUNDLE_TIMESTAMP), - renderingRequest: 'ReactOnRails.getAsyncValue()', - }; - req.write(`${JSON.stringify(initialObject)}\n`); - - // Send a malformed update chunk (missing bundleTimestamp) - const malformedChunk = { - updateChunk: 'ReactOnRails.setAsyncValue("should not work")', - }; - req.write(`${JSON.stringify(malformedChunk)}\n`); - - // Send a valid update chunk after the malformed one - const validChunk = { - bundleTimestamp: SERVER_BUNDLE_TIMESTAMP, - updateChunk: 'ReactOnRails.setAsyncValue("valid update")', - }; - req.write(`${JSON.stringify(validChunk)}\n`); - - // Send a chunk with invalid JavaScript - const invalidJSChunk = { - bundleTimestamp: SERVER_BUNDLE_TIMESTAMP, - updateChunk: 'this is not valid javascript syntax !!!', - }; - req.write(`${JSON.stringify(invalidJSChunk)}\n`); - - // End the request - req.end(); - - // Wait for the response - const response = await responsePromise; - - // Verify the response - should still work despite errors - expect(response.statusCode).toBe(200); - expect(response.data).toBe('"valid update"'); // Should resolve with the valid update - }); - - test('update chunks with non-existent bundle timestamp', async () => { - await createVmBundle(TEST_NAME); - const SERVER_BUNDLE_TIMESTAMP = String(BUNDLE_TIMESTAMP); - const NON_EXISTENT_TIMESTAMP = '9999999999999'; - - // Create the HTTP request - const req = createHttpRequest(SERVER_BUNDLE_TIMESTAMP); - - // Set up response handling - const responsePromise = setupResponseHandler(req, true); - - // Send the initial object - const initialObject = { - ...createInitialObject(SERVER_BUNDLE_TIMESTAMP), - renderingRequest: 'ReactOnRails.getAsyncValue()', - }; - req.write(`${JSON.stringify(initialObject)}\n`); - - // Send update chunk with non-existent bundle timestamp - const updateChunk = { - bundleTimestamp: NON_EXISTENT_TIMESTAMP, - updateChunk: 'ReactOnRails.setAsyncValue("should not work")', - }; - req.write(`${JSON.stringify(updateChunk)}\n`); - - // Send a valid update chunk - const validChunk = { + // End the stream to signal completion + const endStreamChunk = { bundleTimestamp: SERVER_BUNDLE_TIMESTAMP, - updateChunk: 'ReactOnRails.setAsyncValue("valid update")', + updateChunk: 'ReactOnRails.endStream()', }; - req.write(`${JSON.stringify(validChunk)}\n`); + req.write(`${JSON.stringify(endStreamChunk)}\n`); // End the request req.end(); @@ -904,12 +769,13 @@ describe('incremental render NDJSON endpoint', () => { // Verify the response expect(response.statusCode).toBe(200); - expect(response.data).toBe('"valid update"'); // Should resolve with the valid update + // Since we're returning a stream, the response should indicate streaming + expect(streamedData.length).toBeGreaterThan(0); }); test('complex multi-bundle streaming scenario', async () => { - await createVmBundle(TEST_NAME); - await createSecondaryVmBundle(TEST_NAME); + await createIncrementalVmBundle(TEST_NAME); + await createIncrementalSecondaryVmBundle(TEST_NAME); const SERVER_BUNDLE_TIMESTAMP = String(BUNDLE_TIMESTAMP); const SECONDARY_BUNDLE_TIMESTAMP_STR = String(SECONDARY_BUNDLE_TIMESTAMP); diff --git a/react_on_rails_pro/packages/node-renderer/tests/serverRenderRSCReactComponent.test.js b/react_on_rails_pro/packages/node-renderer/tests/serverRenderRSCReactComponent.test.js index b7893e25d2..0c6a95c119 100644 --- a/react_on_rails_pro/packages/node-renderer/tests/serverRenderRSCReactComponent.test.js +++ b/react_on_rails_pro/packages/node-renderer/tests/serverRenderRSCReactComponent.test.js @@ -2,7 +2,8 @@ import path from 'path'; import fs from 'fs'; import { Readable } from 'stream'; import { buildExecutionContext, resetVM } from '../src/worker/vm'; -import { getConfig } from '../src/shared/configBuilder'; +import { buildConfig } from '../src/shared/configBuilder'; +import { serverBundleCachePath } from './helper'; const SimpleWorkingComponent = () => 'hello'; @@ -18,13 +19,14 @@ const ComponentWithAsyncError = async () => { }; describe('serverRenderRSCReactComponent', () => { + const testName = 'serverRenderRSCReactComponent'; let tempDir; let tempRscBundlePath; let tempManifestPath; beforeAll(async () => { - // Create temporary directory - tempDir = path.join(process.cwd(), 'tmp/node-renderer-bundles-test/testing-bundle'); + // Create temporary directory using helper to ensure unique path + tempDir = serverBundleCachePath(testName); fs.mkdirSync(tempDir, { recursive: true }); // Copy rsc-bundle.js to temp directory @@ -49,10 +51,12 @@ describe('serverRenderRSCReactComponent', () => { }); beforeEach(async () => { - const config = getConfig(); - config.supportModules = true; - config.maxVMPoolSize = 2; // Set a small pool size for testing - config.stubTimers = false; + buildConfig({ + serverBundleCachePath: tempDir, + supportModules: true, + stubTimers: false, + maxVMPoolSize: 2, + }); }); afterEach(async () => { diff --git a/react_on_rails_pro/packages/node-renderer/tests/worker.test.ts b/react_on_rails_pro/packages/node-renderer/tests/worker.test.ts index 6dabc5869b..644e7cb4ce 100644 --- a/react_on_rails_pro/packages/node-renderer/tests/worker.test.ts +++ b/react_on_rails_pro/packages/node-renderer/tests/worker.test.ts @@ -483,7 +483,11 @@ describe('worker', () => { // Verify bundles are placed in their correct directories const bundle1Path = path.join(serverBundleCachePathForTest(), bundleHash, `${bundleHash}.js`); - const bundle2Path = path.join(serverBundleCachePathForTest(), secondaryBundleHash, `${secondaryBundleHash}.js`); + const bundle2Path = path.join( + serverBundleCachePathForTest(), + secondaryBundleHash, + `${secondaryBundleHash}.js`, + ); expect(fs.existsSync(bundle1Path)).toBe(true); expect(fs.existsSync(bundle2Path)).toBe(true); @@ -648,8 +652,8 @@ describe('worker', () => { expect(files).toHaveLength(1); expect(files[0]).toBe(`${bundleHash}.js`); - // Verify the original content is preserved (1646 bytes from bundle.js, not 1689 from secondary-bundle.js) - expect(secondBundleSize).toBe(1646); // Size of getFixtureBundle(), not getFixtureSecondaryBundle() + // Verify the original content is preserved (62 bytes from bundle.js, not 84 from secondary-bundle.js) + expect(secondBundleSize).toBe(62); // Size of getFixtureBundle(), not getFixtureSecondaryBundle() }); test('post /upload-assets with bundles placed in their own hash directories, not targetBundles directories', async () => { diff --git a/react_on_rails_pro/spec/dummy/Gemfile.lock b/react_on_rails_pro/spec/dummy/Gemfile.lock index 3d50301024..fec02008bb 100644 --- a/react_on_rails_pro/spec/dummy/Gemfile.lock +++ b/react_on_rails_pro/spec/dummy/Gemfile.lock @@ -22,6 +22,7 @@ PATH specs: react_on_rails_pro (16.2.0.beta.4) addressable + async (>= 2.6) connection_pool execjs (~> 2.9) httpx (~> 1.5) @@ -107,6 +108,12 @@ GEM public_suffix (>= 2.0.2, < 7.0) amazing_print (1.6.0) ast (2.4.2) + async (2.34.0) + console (~> 1.29) + fiber-annotation + io-event (~> 1.11) + metrics (~> 0.12) + traces (~> 0.18) base64 (0.2.0) benchmark (0.4.0) bigdecimal (3.1.9) @@ -131,6 +138,10 @@ GEM coderay (1.1.3) concurrent-ruby (1.3.5) connection_pool (2.5.0) + console (1.34.2) + fiber-annotation + fiber-local (~> 1.1) + json coveralls (0.8.23) json (>= 1.8, < 3) simplecov (~> 0.16.1) @@ -165,6 +176,9 @@ GEM ffi (1.17.0-x86_64-darwin) ffi (1.17.0-x86_64-linux-gnu) ffi (1.17.0-x86_64-linux-musl) + fiber-annotation (0.2.0) + fiber-local (1.1.0) + fiber-storage fiber-storage (1.0.0) generator_spec (0.10.0) activesupport (>= 3.0.0) @@ -184,6 +198,7 @@ GEM i18n (1.14.7) concurrent-ruby (~> 1.0) io-console (0.8.0) + io-event (1.14.0) irb (1.15.1) pp (>= 0.6.0) rdoc (>= 4.0.0) @@ -216,6 +231,7 @@ GEM marcel (1.0.4) matrix (0.4.2) method_source (1.1.0) + metrics (0.15.0) mini_mime (1.1.5) mini_portile2 (2.8.8) minitest (5.25.4) @@ -447,6 +463,7 @@ GEM tins (1.33.0) bigdecimal sync + traces (0.18.2) turbolinks (5.2.1) turbolinks-source (~> 5.2) turbolinks-source (5.2.0) diff --git a/react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb b/react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb index 6c775ae799..495fe9f9e3 100644 --- a/react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb +++ b/react_on_rails_pro/spec/react_on_rails_pro/request_spec.rb @@ -109,6 +109,13 @@ count: 1) do |yielder| yielder.call("Bundle not found\n") end + + # Mock the /upload-assets endpoint that gets called when send_bundle is true + upload_assets_url = "#{renderer_url}/upload-assets" + upload_request_info = mock_streaming_response(upload_assets_url, 200, count: 1) do |yielder| + yielder.call("Assets uploaded\n") + end + second_request_info = mock_streaming_response(render_full_url, 200) do |yielder| yielder.call("Hello, world!\n") end @@ -124,21 +131,33 @@ expect(first_request_info[:request].body.to_s).to include("renderingRequest=console.log") expect(first_request_info[:request].body.to_s).not_to include("bundle") - # Second request should have a bundle - # It's a multipart/form-data request, so we can access the form directly - second_request_body = second_request_info[:request].body.instance_variable_get(:@body) - second_request_form = second_request_body.instance_variable_get(:@form) + # The bundle should be sent via the /upload-assets endpoint + upload_request_body = upload_request_info[:request].body.instance_variable_get(:@body) + upload_request_form = upload_request_body.instance_variable_get(:@form) - expect(second_request_form).to have_key("bundle_server_bundle.js") - expect(second_request_form["bundle_server_bundle.js"][:body]).to be_a(FakeFS::Pathname) - expect(second_request_form["bundle_server_bundle.js"][:body].to_s).to eq(server_bundle_path) + expect(upload_request_form).to have_key("bundle_server_bundle.js") + expect(upload_request_form["bundle_server_bundle.js"][:body]).to be_a(FakeFS::Pathname) + expect(upload_request_form["bundle_server_bundle.js"][:body].to_s).to eq(server_bundle_path) + + # Second render request should also not have a bundle + expect(second_request_info[:request].body.to_s).to include("renderingRequest=console.log") + expect(second_request_info[:request].body.to_s).not_to include("bundle") end it "raises duplicate bundle upload error when server asks for bundle twice" do - first_request_info = mock_streaming_response(render_full_url, ReactOnRailsPro::STATUS_SEND_BUNDLE) do |yielder| + first_request_info = mock_streaming_response(render_full_url, ReactOnRailsPro::STATUS_SEND_BUNDLE, + count: 1) do |yielder| yielder.call("Bundle not found\n") end - second_request_info = mock_streaming_response(render_full_url, ReactOnRailsPro::STATUS_SEND_BUNDLE) do |yielder| + + # Mock the /upload-assets endpoint that gets called when send_bundle is true + upload_assets_url = "#{renderer_url}/upload-assets" + upload_request_info = mock_streaming_response(upload_assets_url, 200, count: 1) do |yielder| + yielder.call("Assets uploaded\n") + end + + second_request_info = mock_streaming_response(render_full_url, ReactOnRailsPro::STATUS_SEND_BUNDLE, + count: 1) do |yielder| yielder.call("Bundle still not found\n") end @@ -153,13 +172,17 @@ expect(first_request_info[:request].body.to_s).to include("renderingRequest=console.log") expect(first_request_info[:request].body.to_s).not_to include("bundle") - # Second request should have a bundle - second_request_body = second_request_info[:request].body.instance_variable_get(:@body) - second_request_form = second_request_body.instance_variable_get(:@form) + # The bundle should be sent via the /upload-assets endpoint + upload_request_body = upload_request_info[:request].body.instance_variable_get(:@body) + upload_request_form = upload_request_body.instance_variable_get(:@form) + + expect(upload_request_form).to have_key("bundle_server_bundle.js") + expect(upload_request_form["bundle_server_bundle.js"][:body]).to be_a(FakeFS::Pathname) + expect(upload_request_form["bundle_server_bundle.js"][:body].to_s).to eq(server_bundle_path) - expect(second_request_form).to have_key("bundle_server_bundle.js") - expect(second_request_form["bundle_server_bundle.js"][:body]).to be_a(FakeFS::Pathname) - expect(second_request_form["bundle_server_bundle.js"][:body].to_s).to eq(server_bundle_path) + # Second render request should also not have a bundle + expect(second_request_info[:request].body.to_s).to include("renderingRequest=console.log") + expect(second_request_info[:request].body.to_s).not_to include("bundle") end it "raises incompatible error when server returns incompatible error" do