sadpandajoe commented on code in PR #45595: URL: https://github.com/apache/superset/pull/45595#discussion_r4371604377
########## superset-frontend/spec/helpers/swcJestTransformer.js: ########## @@ -0,0 +1,134 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { createTransformer as createSwcTransformer } from '@swc/jest'; + +/** + * Jest transformer: @swc/jest plus one post-processing step. + * + * SWC's CommonJS output defines every ESM export as a lazy, non-configurable + * getter, which is why `jest.spyOn(module, 'name')` and + * `jest.replaceProperty(module, 'name', value)` throw "Cannot redefine + * property" on anything SWC transformed. Babel's output defined plain + * properties, so the suite relies on spying on module exports everywhere. + * This wrapper marks those getters `configurable: true` so the spies keep + * working, while leaving the getters lazy (an eager rewrite would break + * modules that import each other in a cycle). + * + * The second difference is what a module in an import cycle sees when it + * reads an export that has not been initialised yet. Babel assigns + * `exports.name = void 0` up front, so the read yields `undefined`; SWC's + * getter returns the live binding and throws a ReferenceError (temporal dead + * zone) instead. Several suites spread `jest.requireActual()` of a module + * that is still mid-evaluation, so the getters are made lenient to match + * Babel: an uninitialised export reads as `undefined`. + */ +// SWC keeps a leading doc comment of a default export inside the call, as +// `Object.defineProperty(exports, /** ... */ "default", {`, hence the optional +// block comment before the name. +const EXPORT_GETTER_DESCRIPTOR = + /(Object\.defineProperty\((?:target, name|to, k|exports,\s*(?:\/\*[\s\S]*?\*\/\s*)?"[^"]+"), \{\s*enumerable: true,)(\s*)(get:)/g; +// `_export(exports, { get name () { return binding; }, ... });` is how SWC +// declares named exports; the leniency below is scoped to this block and to +// the `Object.defineProperty(exports, ...)` descriptors, never to getters in +// application code. +const EXPORT_BLOCK = /_export\(exports, \{[\s\S]*?\n\}\);/g; +const EXPORT_BLOCK_GETTER = /(get \w+ \(\) \{\s*)return (\w+);/g; +// The `get: function() { return binding; }` form SWC uses for a single export +// or a re-export (`from[k]`), matched together with its descriptor. +const EXPORT_DESCRIPTOR_GETTER = + /(Object\.defineProperty\((?:target, name|to, k|exports,\s*(?:\/\*[\s\S]*?\*\/\s*)?"[^"]+"), \{\s*enumerable: true,\s*)(get: function\(\) \{\s*)return (\w+|from\[k\]);/g; + +// Only a TDZ ReferenceError is swallowed; any other error still propagates. +// The istanbul hint keeps the injected try/catch out of coverage instrumentation +// so exported bindings do not report uncovered branches. +const lenient = binding => + `/* istanbul ignore next */ try { return ${binding}; } catch (e) { if (!(e instanceof ReferenceError)) throw e; return undefined; }`; + +function rewriteExports(code) { + return code + .replace( + EXPORT_DESCRIPTOR_GETTER, + (_m, head, getter, binding) => `${head}${getter}${lenient(binding)}`, + ) + .replace(EXPORT_BLOCK, block => + block.replace( + EXPORT_BLOCK_GETTER, + (_m, head, binding) => `${head}${lenient(binding)}`, + ), + ) + .replace(EXPORT_GETTER_DESCRIPTOR, '$1 configurable: true,$2$3'); +} + +export function mirrorBabelExportSemantics(code) { + const patched = rewriteExports(code); + // A shape change in SWC's export helpers would otherwise leave exports + // non-configurable, surfacing later as scattered "Cannot redefine property" + // errors or TDZ ReferenceErrors in unrelated suites. + if ( + /\b_export\(/.test(code) && + /enumerable: true,\s*get\b/.test(code) && + !patched.includes('configurable: true') + ) { + throw new Error( + 'swcJestTransformer: SWC export output did not match any known shape; ' + + 'update the patterns in spec/helpers/swcJestTransformer.js', + ); + } + // The helper descriptor always supplies `configurable: true`, so a changed + // named-getter shape would otherwise lose only the leniency half unnoticed. + if ( + /\b_export\(exports, \{[\s\S]*?\bget\s+\w+\s*\(/.test(code) && Review Comment: This new guard throws on any module whose `_export` getters return a member expression. A barrel like `packages/superset-ui-core/src/index.ts` re-exports with getters such as `return _foo.bar;`, but `EXPORT_BLOCK_GETTER` only rewrites a bare `\w+` binding, so nothing gets a `try { return` and the guard throws "named export getters did not match". The failing `sharded-jest-tests` shards at this head show it (e.g. `src/utils/downloadAsImage.test.ts` fails to run, with the stack going through `superset-ui-core/src/index.ts`), so every suite that imports that package fails to load. The same regex also skips `$`-containing names, so a block mixing `$value` and a plain name would pass the guard while leaving `$value` unwrapped. Should the getter pattern accept member-expression and `$` identifiers (or the guard check only the shapes the rewrite actually supports)? ########## superset-frontend/spec/helpers/swcJestTransformer.test.js: ########## @@ -0,0 +1,85 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { mirrorBabelExportSemantics } from './swcJestTransformer'; + +const NAMED_EXPORTS = `"use strict"; +Object.defineProperty(exports, "__esModule", { + value: true +}); +function _export(target, all) { + for(var name in all)Object.defineProperty(target, name, { + enumerable: true, + get: Object.getOwnPropertyDescriptor(all, name).get + }); +} +_export(exports, { + get answer () { + return answer; + } +}); +const answer = 42; +`; + +function load(code) { + const mod = { exports: {} }; + // eslint-disable-next-line no-new-func + new Function('module', 'exports', code)(mod, mod.exports); + return mod.exports; +} + +test('named exports become configurable so jest.spyOn works', () => { + const exports = load(mirrorBabelExportSemantics(NAMED_EXPORTS)); Review Comment: This test file goes through the same transformer when Jest loads it, and the `NAMED_EXPORTS` template literal contains `_export(exports, {...})` and the helper's `Object.defineProperty(target, name, {` text, so the fixture is already rewritten (configurable, `try` getter) before `mirrorBabelExportSemantics` is called on it here. That means the configurable and uninitialised-export assertions at lines 48-51 and 54-62 still pass if `mirrorBabelExportSemantics` becomes a no-op, so a regression in the rewrite would not fail them. Could the fixture be built so the transformer cannot see the export-helper text (as the last test already does for `get`), so these tests exercise raw SWC-shaped input? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
