From 8c5734965bf93a2f0c3d531a18be0adaba84ad84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Thu, 27 Jan 2022 15:22:35 +0100 Subject: [PATCH] fix: allow certain variable names in development (#33638) Fixes #24570 A few variable names (listed https://github.com/vercel/next.js/issues/24570#issuecomment-828721019) were causing problems when using `next dev`. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `yarn lint` --- .../internal/ReactRefreshModule.runtime.ts | 115 +++++++++--------- .../acceptance/ReactRefreshModule.test.ts | 44 +++++++ 2 files changed, 103 insertions(+), 56 deletions(-) create mode 100644 test/development/acceptance/ReactRefreshModule.test.ts diff --git a/packages/react-refresh-utils/internal/ReactRefreshModule.runtime.ts b/packages/react-refresh-utils/internal/ReactRefreshModule.runtime.ts index 3085febfb7fd0c7..d4b5814bc6eb8ee 100644 --- a/packages/react-refresh-utils/internal/ReactRefreshModule.runtime.ts +++ b/packages/react-refresh-utils/internal/ReactRefreshModule.runtime.ts @@ -17,67 +17,70 @@ declare const module: { // This function gets unwrapped into global scope, which is why we don't invert // if-blocks. Also, you cannot use `return`. export default function () { - // Legacy CSS implementations will `eval` browser code in a Node.js context - // to extract CSS. For backwards compatibility, we need to check we're in a - // browser context before continuing. - if ( - typeof self !== 'undefined' && - // AMP / No-JS mode does not inject these helpers: - '$RefreshHelpers$' in self - ) { - var currentExports = module.__proto__.exports - var prevExports = module.hot.data?.prevExports ?? null + // Wrapped in an IIFE to avoid polluting the global scope + ;(function () { + // Legacy CSS implementations will `eval` browser code in a Node.js context + // to extract CSS. For backwards compatibility, we need to check we're in a + // browser context before continuing. + if ( + typeof self !== 'undefined' && + // AMP / No-JS mode does not inject these helpers: + '$RefreshHelpers$' in self + ) { + var currentExports = module.__proto__.exports + var prevExports = module.hot.data?.prevExports ?? null - // This cannot happen in MainTemplate because the exports mismatch between - // templating and execution. - self.$RefreshHelpers$.registerExportsForReactRefresh( - currentExports, - module.id - ) + // This cannot happen in MainTemplate because the exports mismatch between + // templating and execution. + self.$RefreshHelpers$.registerExportsForReactRefresh( + currentExports, + module.id + ) - // A module can be accepted automatically based on its exports, e.g. when - // it is a Refresh Boundary. - if (self.$RefreshHelpers$.isReactRefreshBoundary(currentExports)) { - // Save the previous exports on update so we can compare the boundary - // signatures. - module.hot.dispose(function (data) { - data.prevExports = currentExports - }) - // Unconditionally accept an update to this module, we'll check if it's - // still a Refresh Boundary later. - module.hot.accept() + // A module can be accepted automatically based on its exports, e.g. when + // it is a Refresh Boundary. + if (self.$RefreshHelpers$.isReactRefreshBoundary(currentExports)) { + // Save the previous exports on update so we can compare the boundary + // signatures. + module.hot.dispose(function (data) { + data.prevExports = currentExports + }) + // Unconditionally accept an update to this module, we'll check if it's + // still a Refresh Boundary later. + module.hot.accept() - // This field is set when the previous version of this module was a - // Refresh Boundary, letting us know we need to check for invalidation or - // enqueue an update. - if (prevExports !== null) { - // A boundary can become ineligible if its exports are incompatible - // with the previous exports. - // - // For example, if you add/remove/change exports, we'll want to - // re-execute the importing modules, and force those components to - // re-render. Similarly, if you convert a class component to a - // function, we want to invalidate the boundary. - if ( - self.$RefreshHelpers$.shouldInvalidateReactRefreshBoundary( - prevExports, - currentExports - ) - ) { + // This field is set when the previous version of this module was a + // Refresh Boundary, letting us know we need to check for invalidation or + // enqueue an update. + if (prevExports !== null) { + // A boundary can become ineligible if its exports are incompatible + // with the previous exports. + // + // For example, if you add/remove/change exports, we'll want to + // re-execute the importing modules, and force those components to + // re-render. Similarly, if you convert a class component to a + // function, we want to invalidate the boundary. + if ( + self.$RefreshHelpers$.shouldInvalidateReactRefreshBoundary( + prevExports, + currentExports + ) + ) { + module.hot.invalidate() + } else { + self.$RefreshHelpers$.scheduleUpdate() + } + } + } else { + // Since we just executed the code for the module, it's possible that the + // new exports made it ineligible for being a boundary. + // We only care about the case when we were _previously_ a boundary, + // because we already accepted this update (accidental side effect). + var isNoLongerABoundary = prevExports !== null + if (isNoLongerABoundary) { module.hot.invalidate() - } else { - self.$RefreshHelpers$.scheduleUpdate() } } - } else { - // Since we just executed the code for the module, it's possible that the - // new exports made it ineligible for being a boundary. - // We only care about the case when we were _previously_ a boundary, - // because we already accepted this update (accidental side effect). - var isNoLongerABoundary = prevExports !== null - if (isNoLongerABoundary) { - module.hot.invalidate() - } } - } + })() } diff --git a/test/development/acceptance/ReactRefreshModule.test.ts b/test/development/acceptance/ReactRefreshModule.test.ts new file mode 100644 index 000000000000000..6dae853adc77c52 --- /dev/null +++ b/test/development/acceptance/ReactRefreshModule.test.ts @@ -0,0 +1,44 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { sandbox } from './helpers' + +describe('ReactRefreshModule', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: {}, + skipStart: true, + }) + }) + afterAll(() => next.destroy()) + + it('should allow any variable names', async () => { + const { session, cleanup } = await sandbox(next, new Map([])) + expect(await session.hasRedbox()).toBe(false) + + const variables = [ + '_a', + '_b', + 'currentExports', + 'prevExports', + 'isNoLongerABoundary', + ] + + for await (const variable of variables) { + await session.patch( + 'pages/index.js', + `import { default as ${variable} } from 'next/link' + export default function Page() { + return null + }` + ) + expect(await session.hasRedbox()).toBe(false) + expect(next.cliOutput).not.toContain( + `'${variable}' has already been declared` + ) + } + + await cleanup() + }) +})