Skip to content

Commit

Permalink
fix: limit the number of globals defined due to the v8 snapshot (#25051)
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanthemanuel authored Dec 8, 2022
1 parent c540284 commit f9541af
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 112 deletions.
10 changes: 5 additions & 5 deletions .circleci/workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ mainBuildFilters: &mainBuildFilters
only:
- develop
- /^release\/\d+\.\d+\.\d+$/
- 'ryanm/fix/cy-in-cy-and-v8-snapshots'
- 'ryanm/fix/issue-with-integrity-check'

# usually we don't build Mac app - it takes a long time
# but sometimes we want to really confirm we are doing the right thing
Expand All @@ -37,15 +37,15 @@ macWorkflowFilters: &darwin-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/cy-in-cy-and-v8-snapshots', << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/issue-with-integrity-check', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/cy-in-cy-and-v8-snapshots', << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/issue-with-integrity-check', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
Expand All @@ -63,7 +63,7 @@ windowsWorkflowFilters: &windows-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/cy-in-cy-and-v8-snapshots', << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/issue-with-integrity-check', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
Expand Down Expand Up @@ -129,7 +129,7 @@ commands:
- run:
name: Check current branch to persist artifacts
command: |
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "ryanm/fix/cy-in-cy-and-v8-snapshots" ]]; then
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "ryanm/fix/issue-with-integrity-check" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
Expand Down
6 changes: 3 additions & 3 deletions packages/frontend-shared/cypress/e2e/e2ePluginSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ chai.use(chaiSubset)
chai.use(sinonChai)

export async function e2ePluginSetup (on: Cypress.PluginEvents, config: Cypress.PluginConfigOptions) {
// @ts-ignore snapshotAuxiliaryData is injected by the snapshot script
if (typeof global.snapshotAuxiliaryData === 'undefined') {
throw new Error('snapshotAuxiliaryData is undefined. v8 snapshots are not being used in Cypress in Cypress')
// @ts-ignore getSnapshotResult is injected by the snapshot script
if (typeof global.getSnapshotResult === 'undefined') {
throw new Error('getSnapshotResult is undefined. v8 snapshots are not being used in Cypress in Cypress')
}

process.env.CYPRESS_INTERNAL_E2E_TESTING_SELF = 'true'
Expand Down
24 changes: 10 additions & 14 deletions packages/v8-snapshot-require/src/snapshot-require.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,20 +207,16 @@ export function snapshotRequire (
let moduleNeedsReload: ModuleNeedsReload | undefined

// @ts-ignore global snapshotAuxiliaryData
if (typeof snapshotAuxiliaryData !== 'undefined') {
// @ts-ignore global snapshotAuxiliaryData
resolverMap = snapshotAuxiliaryData.resolverMap
const dependencyMapArray: DependencyMapArray =
// @ts-ignore global snapshotAuxiliaryData
snapshotAuxiliaryData.dependencyMapArray

// 5. Setup the module needs reload predicate with the dependency map
if (dependencyMapArray != null) {
moduleNeedsReload = createModuleNeedsReload(
dependencyMapArray,
projectBaseDir,
)
}
resolverMap = sr.snapshotAuxiliaryData.resolverMap
// @ts-ignore global snapshotAuxiliaryData
const dependencyMapArray: DependencyMapArray = sr.snapshotAuxiliaryData.dependencyMapArray

// 5. Setup the module needs reload predicate with the dependency map
if (dependencyMapArray != null) {
moduleNeedsReload = createModuleNeedsReload(
dependencyMapArray,
projectBaseDir,
)
}

// 6. Setup the module key resolver with the resolver map
Expand Down
15 changes: 15 additions & 0 deletions scripts/binary/smoke.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,21 @@ const runIntegrityTest = async function (buildAppExecutable, buildAppDir, e2e) {

await testAlteringEntryPoint('console.log("simple alteration")', 'Integrity check failed with expected stack length 9 but got 10')
await testAlteringEntryPoint('console.log("accessing " + global.getSnapshotResult())', 'getSnapshotResult can only be called once')

function compareGlobals () {
const childProcess = require('child_process')
const nodeGlobalKeys = JSON.parse(childProcess.spawnSync('node -p "const x = Object.getOwnPropertyNames(global);JSON.stringify(x)"', { shell: true, encoding: 'utf8' }).stdout)

const extraKeys = Object.getOwnPropertyNames(global).filter((key) => {
return !nodeGlobalKeys.includes(key)
})

console.error(`extra keys in electron process: ${extraKeys}`)
}

const allowList = ['regeneratorRuntime', '__core-js_shared__', 'getSnapshotResult', 'supportTypeScript']

await testAlteringEntryPoint(`(${compareGlobals.toString()})()`, `extra keys in electron process: ${allowList}\nIntegrity check failed with expected stack length 9 but got 10`)
}

const test = async function (buildAppExecutable, buildAppDir) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
describe('simple v8 snapshot spec', () => {
it('passes', () => {
// Snapshot result needs to be undefined in the renderer process
expect(window.snapshotResult).to.be.undefined
expect(window.getSnapshotResult).to.be.undefined
})
})
167 changes: 88 additions & 79 deletions tooling/v8-snapshot/src/generator/blueprint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,101 +109,110 @@ export function scriptFromBlueprint (config: BlueprintConfig): {

const wrapperOpen = Buffer.from(
`
const PATH_SEP = '${pathSep}'
var snapshotAuxiliaryData = ${auxiliaryData}
${integrityCheckSource || ''}
function generateSnapshot() {
//
// <process>
//
function cannotAccess(proto, prop) {
return function () {
throw 'Cannot access ' + proto + '.' + prop + ' during snapshot creation'
(function () {
const PATH_SEP = '${pathSep}'
${integrityCheckSource || ''}
function generateSnapshot() {
//
// <process>
//
function cannotAccess(proto, prop) {
return function () {
throw 'Cannot access ' + proto + '.' + prop + ' during snapshot creation'
}
}
}
function getPrevent(proto, prop) {
return {
get: cannotAccess(proto, prop)
function getPrevent(proto, prop) {
return {
get: cannotAccess(proto, prop)
}
}
}
let process = {}
Object.defineProperties(process, {
platform: {
value: '${processPlatform}',
enumerable: false,
},
argv: {
value: [],
enumerable: false,
},
env: {
value: {
NODE_ENV: '${nodeEnv}',
let process = {}
Object.defineProperties(process, {
platform: {
value: '${processPlatform}',
enumerable: false,
},
enumerable: false,
},
version: {
value: '${processNodeVersion}',
enumerable: false,
},
versions: {
value: { node: '${processNodeVersion}' },
enumerable: false,
},
nextTick: getPrevent('process', 'nextTick')
})
argv: {
value: [],
enumerable: false,
},
env: {
value: {
NODE_ENV: '${nodeEnv}',
},
enumerable: false,
},
version: {
value: '${processNodeVersion}',
enumerable: false,
},
versions: {
value: { node: '${processNodeVersion}' },
enumerable: false,
},
nextTick: getPrevent('process', 'nextTick')
})
function get_process() {
return process
}
//
// </process>
//
function get_process() {
return process
}
//
// </process>
//
${globals}
${includeStrictVerifiers ? strictGlobals : ''}
${globals}
${includeStrictVerifiers ? strictGlobals : ''}
`,
'utf8',
)
const wrapperClose = Buffer.from(
`
${customRequire}
${includeStrictVerifiers ? 'require.isStrict = true' : ''}
`
${customRequire}
${includeStrictVerifiers ? 'require.isStrict = true' : ''}
customRequire(${normalizedMainModuleRequirePath}, ${normalizedMainModuleRequirePath})
const result = {}
Object.defineProperties(result, {
customRequire: {
writable: false,
value: customRequire
},
setGlobals: {
writable: false,
value: ${setGlobals}
},
snapshotAuxiliaryData: {
writable: false,
value: ${auxiliaryData},
},
})
return result
}
customRequire(${normalizedMainModuleRequirePath}, ${normalizedMainModuleRequirePath})
const result = {}
Object.defineProperties(result, {
customRequire: {
let numberOfGetSnapshotResultCalls = 0
const snapshotResult = generateSnapshot.call({})
Object.defineProperties(this, {
getSnapshotResult: {
writable: false,
value: customRequire
value: function () {
if (numberOfGetSnapshotResultCalls > 0) {
throw new Error('getSnapshotResult can only be called once')
}
numberOfGetSnapshotResultCalls++
return snapshotResult
},
},
setGlobals: {
supportTypeScript: {
writable: false,
value: ${setGlobals}
}
value: ${supportTypeScript},
},
})
return result
}
let numberOfGetSnapshotResultCalls = 0
const snapshotResult = generateSnapshot.call({})
Object.defineProperty(this, 'getSnapshotResult', {
writable: false,
value: function () {
if (numberOfGetSnapshotResultCalls > 0) {
throw new Error('getSnapshotResult can only be called once')
}
numberOfGetSnapshotResultCalls++
return snapshotResult
},
})
var supportTypeScript = ${supportTypeScript}
generateSnapshot = null
generateSnapshot = null
}).call(this)
`,
'utf8',
'utf8',
)

const buffers = [wrapperOpen, customRequireDefinitions, wrapperClose]
Expand Down
11 changes: 1 addition & 10 deletions tooling/v8-snapshot/test/utils/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,5 @@ export function readSnapshotResult (cacheDir: string) {
const metaFile = path.join(cacheDir, 'snapshot-meta.json')
const meta = fs.readJSONSync(metaFile)

const snapshotFile = path.join(cacheDir, 'snapshot.js')

const snapshotFileContent = fs.readFileSync(snapshotFile, 'utf8')
const sourcemapComment = snapshotFileContent.split('\n').pop()

const { snapshotResult, snapshotAuxiliaryData } = eval(
`(function () {\n${snapshotFileContent};\n return { snapshotResult, snapshotAuxiliaryData };}).bind({})()`,
)

return { meta, snapshotResult, snapshotAuxiliaryData, sourcemapComment }
return { meta }
}

5 comments on commit f9541af

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on f9541af Dec 8, 2022

Choose a reason for hiding this comment

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

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.0.2/linux-arm64/develop-f9541aff10efa62ffdd15f34775c2761ba78adf3/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on f9541af Dec 8, 2022

Choose a reason for hiding this comment

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

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.0.2/linux-x64/develop-f9541aff10efa62ffdd15f34775c2761ba78adf3/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on f9541af Dec 8, 2022

Choose a reason for hiding this comment

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

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.0.2/darwin-arm64/develop-f9541aff10efa62ffdd15f34775c2761ba78adf3/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on f9541af Dec 8, 2022

Choose a reason for hiding this comment

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

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.0.2/darwin-x64/develop-f9541aff10efa62ffdd15f34775c2761ba78adf3/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on f9541af Dec 8, 2022

Choose a reason for hiding this comment

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

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.0.2/win32-x64/develop-f9541aff10efa62ffdd15f34775c2761ba78adf3/cypress.tgz

Please sign in to comment.