From 4c1c4fa189cab9c6d38c748df40a88701d07e0a2 Mon Sep 17 00:00:00 2001 From: Anton Evzhakov Date: Thu, 16 Feb 2023 19:08:29 +0200 Subject: [PATCH] fix(babel): modules being erroneously evaluated as empty objects (fixes #1209) --- .changeset/sour-taxis-hear.md | 7 ++ packages/babel/src/index.ts | 2 +- packages/babel/src/module.ts | 96 +++++----------- .../transform-stages/1-prepare-for-eval.ts | 22 +++- packages/testkit/src/babel.test.ts | 4 - .../src/collectExportsAndImports.test.ts | 22 ++++ packages/testkit/src/module.test.ts | 104 ++++++++++++++++-- packages/utils/src/asyncResolveFallback.ts | 8 +- .../utils/src/collectExportsAndImports.ts | 44 ++++++-- 9 files changed, 212 insertions(+), 97 deletions(-) create mode 100644 .changeset/sour-taxis-hear.md diff --git a/.changeset/sour-taxis-hear.md b/.changeset/sour-taxis-hear.md new file mode 100644 index 000000000..767fdff30 --- /dev/null +++ b/.changeset/sour-taxis-hear.md @@ -0,0 +1,7 @@ +--- +'@linaria/babel-preset': patch +'@linaria/testkit': patch +'@linaria/utils': patch +--- + +Address the problem in which a module may be erroneously evaluated as an empty object (fixes #1209) diff --git a/packages/babel/src/index.ts b/packages/babel/src/index.ts index 9e840c111..e4f2c8124 100644 --- a/packages/babel/src/index.ts +++ b/packages/babel/src/index.ts @@ -17,7 +17,7 @@ export { default as preeval } from './plugins/preeval'; export * from './utils/collectTemplateDependencies'; export { default as collectTemplateDependencies } from './utils/collectTemplateDependencies'; export { default as withLinariaMetadata } from './utils/withLinariaMetadata'; -export { default as Module } from './module'; +export { default as Module, DefaultModuleImplementation } from './module'; export { default as transform } from './transform'; export * from './types'; export { default as loadLinariaOptions } from './transform-stages/helpers/loadLinariaOptions'; diff --git a/packages/babel/src/module.ts b/packages/babel/src/module.ts index 9593b6005..87edc72f8 100644 --- a/packages/babel/src/module.ts +++ b/packages/babel/src/module.ts @@ -28,6 +28,18 @@ import { TransformCacheCollection } from './cache'; import * as process from './process'; import type { ITransformFileResult } from './types'; +type HiddenModuleMembers = { + _extensions: { [key: string]: () => void }; + _nodeModulePaths(filename: string): string[]; + _resolveFilename: ( + id: string, + options: { id: string; filename: string; paths: string[] } + ) => string; +}; + +export const DefaultModuleImplementation = NativeModule as typeof NativeModule & + HiddenModuleMembers; + // Supported node builtins based on the modules polyfilled by webpack // `true` means module is polyfilled, `false` means module is empty const builtins = { @@ -87,23 +99,10 @@ const hasKey = ( key in obj; class Module { - static invalidate: () => void; - - static invalidateEvalCache: () => void; - - static _resolveFilename: ( - id: string, - options: { id: string; filename: string; paths: string[] } - ) => string; - - static _nodeModulePaths: (filename: string) => string[]; - #isEvaluated = false; #exports: Record | unknown; - // #exportsProxy: Record; - #lazyValues: Map unknown>; readonly idx: number; @@ -116,7 +115,7 @@ class Module { imports: Map | null; - paths: string[]; + // paths: string[]; extensions: string[]; @@ -146,14 +145,14 @@ class Module { options: StrictOptions, cache = new TransformCacheCollection(), private debuggerDepth = 0, - private parentModule?: Module + private parentModule?: Module, + private moduleImpl: HiddenModuleMembers = DefaultModuleImplementation ) { this.idx = getFileIdx(filename); this.id = filename; this.filename = filename; this.options = options; this.imports = null; - this.paths = []; this.dependencies = null; this.transform = null; this.debug = createCustomDebug('module', this.idx); @@ -162,27 +161,6 @@ class Module { this.#codeCache = cache.codeCache; this.#evalCache = cache.evalCache; - Object.defineProperties(this, { - id: { - value: filename, - writable: false, - }, - filename: { - value: filename, - writable: false, - }, - paths: { - value: Object.freeze( - ( - NativeModule as unknown as { - _nodeModulePaths(filename: string): string[]; - } - )._nodeModulePaths(path.dirname(filename)) - ), - writable: false, - }, - }); - this.#lazyValues = new Map(); const exports: Record = {}; @@ -305,11 +283,7 @@ class Module { return this.#resolveCache.get(resolveCacheKey)!; } - const extensions = ( - NativeModule as unknown as { - _extensions: { [key: string]: () => void }; - } - )._extensions; + const extensions = this.moduleImpl._extensions; const added: string[] = []; try { @@ -326,7 +300,13 @@ class Module { added.push(ext); }); - return Module._resolveFilename(id, this); + const { filename } = this; + + return this.moduleImpl._resolveFilename(id, { + id: filename, + filename, + paths: this.moduleImpl._nodeModulePaths(path.dirname(filename)), + }); } finally { // Cleanup the extensions we added to restore previous behaviour added.forEach((ext) => delete extensions[ext]); @@ -399,10 +379,10 @@ class Module { if (extension === '.json' || this.extensions.includes(extension)) { let code: string | undefined; // Requested file can be already prepared for evaluation on the stage 1 - if (this.#codeCache.has(filename)) { + if (onlyList && this.#codeCache.has(filename)) { const cached = this.#codeCache.get(filename); const only = onlyList - ?.split(',') + .split(',') .filter((token) => !m.#lazyValues.has(token)); const cachedOnly = new Set(cached?.only ?? []); const isMatched = @@ -427,7 +407,10 @@ class Module { } else { // If code wasn't extracted from cache, read it from the file system // TODO: transpile the file - m.debug('code-cache', '❌'); + m.debug( + 'code-cache', + '❌ file has not been processed during prepare stage' + ); code = fs.readFileSync(filename, 'utf-8'); } @@ -519,25 +502,4 @@ class Module { } } -Module.invalidate = () => {}; - -Module.invalidateEvalCache = () => {}; - -// Alias to resolve the module using node's resolve algorithm -// This static property can be overriden by the webpack loader -// This allows us to use webpack's module resolution algorithm -Module._resolveFilename = (id, options) => - ( - NativeModule as unknown as { - _resolveFilename: typeof Module._resolveFilename; - } - )._resolveFilename(id, options); - -Module._nodeModulePaths = (filename: string) => - ( - NativeModule as unknown as { - _nodeModulePaths: (filename: string) => string[]; - } - )._nodeModulePaths(filename); - export default Module; diff --git a/packages/babel/src/transform-stages/1-prepare-for-eval.ts b/packages/babel/src/transform-stages/1-prepare-for-eval.ts index f1a6b9a72..2434fae2d 100644 --- a/packages/babel/src/transform-stages/1-prepare-for-eval.ts +++ b/packages/babel/src/transform-stages/1-prepare-for-eval.ts @@ -285,11 +285,27 @@ export function prepareForEvalSync( for (const [importedFile, importsOnly] of imports ?? []) { try { const resolved = resolve(importedFile, name, resolveStack); - log('stage-1:sync-resolve', `✅ ${importedFile} -> ${resolved}`); + log( + 'stage-1:sync-resolve', + `✅ ${importedFile} -> ${resolved} (only: %o)`, + importsOnly + ); + + const resolveCacheKey = `${name} -> ${importedFile}`; + const resolveCached = cache.resolveCache.get(resolveCacheKey); + const importsOnlySet = new Set(importsOnly); + if (resolveCached) { + const [, cachedOnly] = resolveCached.split('\0'); + cachedOnly?.split(',').forEach((token) => { + importsOnlySet.add(token); + }); + } + cache.resolveCache.set( - `${name} -> ${importedFile}`, - `${resolved}\0${importsOnly.join(',')}` + resolveCacheKey, + `${resolved}\0${[...importsOnlySet].join(',')}` ); + const fileContent = readFileSync(resolved, 'utf8'); queue.enqueue([ { diff --git a/packages/testkit/src/babel.test.ts b/packages/testkit/src/babel.test.ts index 001569884..d56368b4d 100644 --- a/packages/testkit/src/babel.test.ts +++ b/packages/testkit/src/babel.test.ts @@ -8,7 +8,6 @@ import stripAnsi from 'strip-ansi'; import type { PluginOptions, Stage } from '@linaria/babel-preset'; import { - Module, transform as linariaTransform, loadLinariaOptions, } from '@linaria/babel-preset'; @@ -139,9 +138,6 @@ async function transformFile(filename: string, opts: Options) { describe('strategy shaker', () => { const evaluator = require('@linaria/shaker').default; - beforeEach(() => { - Module.invalidateEvalCache(); - }); it('transpiles styled template literal with object', async () => { const { code, metadata } = await transform( diff --git a/packages/testkit/src/collectExportsAndImports.test.ts b/packages/testkit/src/collectExportsAndImports.test.ts index fa3a28ce0..425074156 100644 --- a/packages/testkit/src/collectExportsAndImports.test.ts +++ b/packages/testkit/src/collectExportsAndImports.test.ts @@ -734,6 +734,28 @@ describe.each(compilers)('collectExportsAndImports (%s)', (name, compiler) => { } }); + it('__exportStar', () => { + const { exports, imports, reexports } = run` + const tslib_1 = require('tslib'); + tslib_1.__exportStar(require('./moduleA1'), exports); + `; + + expect(reexports.map(withoutLocal)).toMatchObject([ + { + imported: '*', + exported: '*', + source: './moduleA1', + }, + ]); + expect(exports).toHaveLength(0); + expect(imports).toMatchObject([ + { + source: 'tslib', + imported: '__exportStar', + }, + ]); + }); + it('mixed exports', () => { const { exports, imports, reexports } = run` export { syncResolve } from './asyncResolveFallback'; diff --git a/packages/testkit/src/module.test.ts b/packages/testkit/src/module.test.ts index 5763aca04..df599d8ec 100644 --- a/packages/testkit/src/module.test.ts +++ b/packages/testkit/src/module.test.ts @@ -2,11 +2,13 @@ import path from 'path'; import dedent from 'dedent'; -import { Module, TransformCacheCollection } from '@linaria/babel-preset'; +import { + DefaultModuleImplementation, + Module, + TransformCacheCollection, +} from '@linaria/babel-preset'; import type { StrictOptions } from '@linaria/utils'; -beforeEach(() => Module.invalidate()); - function getFileName() { return path.resolve(__dirname, './__fixtures__/test.js'); } @@ -19,8 +21,6 @@ const options: StrictOptions = { babelOptions: {}, }; -beforeEach(() => Module.invalidateEvalCache()); - it('creates module for JS files', () => { const filename = '/foo/bar/test.js'; const mod = new Module(filename, options); @@ -82,6 +82,61 @@ it('returns module from the cache', () => { expect(res1).toBe(res2); }); +it('should use cached version from the codeCache', () => { + const filename = getFileName(); + const cache = new TransformCacheCollection(); + const mod = new Module(filename, options, cache); + const resolved = require.resolve('./__fixtures__/objectExport.js'); + + cache.resolveCache.set( + `${filename} -> ./objectExport`, + `${resolved}\0margin` + ); + + cache.codeCache.set(resolved, { + only: ['margin'], + imports: null, + result: { + code: 'module.exports = { margin: 1 };', + }, + }); + + mod.evaluate(dedent` + const margin = require('./objectExport').margin; + + module.exports = 'Imported value is ' + margin; + `); + + expect(mod.exports).toBe('Imported value is 1'); +}); + +it('should reread module from disk when it is in codeCache but not in resolveCache', () => { + // This may happen when the current importer was not processed, but required + // module was already required by another module, and its code was cached. + // In this case, we should not use the cached code, but reread the file. + + const filename = getFileName(); + const cache = new TransformCacheCollection(); + const mod = new Module(filename, options, cache); + const resolved = require.resolve('./__fixtures__/objectExport.js'); + + cache.codeCache.set(resolved, { + only: ['margin'], + imports: null, + result: { + code: 'module.exports = { margin: 1 };', + }, + }); + + mod.evaluate(dedent` + const margin = require('./objectExport').margin; + + module.exports = 'Imported value is ' + margin; + `); + + expect(mod.exports).toBe('Imported value is 5'); +}); + it('clears modules from the cache', () => { const filename = getFileName(); const cache = new TransformCacheCollection(); @@ -259,9 +314,9 @@ it('has global objects available without referencing global', () => { }); it('changes resolve behaviour on overriding _resolveFilename', () => { - const originalResolveFilename = Module._resolveFilename; - - Module._resolveFilename = (id) => (id === 'foo' ? 'bar' : id); + const resolveFilename = jest + .spyOn(DefaultModuleImplementation, '_resolveFilename') + .mockImplementation((id) => (id === 'foo' ? 'bar' : id)); const mod = new Module(getFileName(), options); @@ -272,10 +327,37 @@ it('changes resolve behaviour on overriding _resolveFilename', () => { ]; `); - // Restore old behavior - Module._resolveFilename = originalResolveFilename; - expect(mod.exports).toEqual(['bar', 'test']); + expect(resolveFilename).toHaveBeenCalledTimes(2); + + resolveFilename.mockRestore(); +}); + +it('should resolve from the cache', () => { + const resolveFilename = jest.spyOn( + DefaultModuleImplementation, + '_resolveFilename' + ); + + const cache = new TransformCacheCollection(); + const filename = getFileName(); + + cache.resolveCache.set(`${filename} -> foo`, 'resolved foo'); + cache.resolveCache.set(`${filename} -> test`, 'resolved test'); + + const mod = new Module(filename, options, cache); + + mod.evaluate(dedent` + module.exports = [ + require.resolve('foo'), + require.resolve('test'), + ]; + `); + + expect(mod.exports).toEqual(['resolved foo', 'resolved test']); + expect(resolveFilename).toHaveBeenCalledTimes(0); + + resolveFilename.mockRestore(); }); it('correctly processes export declarations in strict mode', () => { diff --git a/packages/utils/src/asyncResolveFallback.ts b/packages/utils/src/asyncResolveFallback.ts index b6ce583a0..bc1feb68e 100644 --- a/packages/utils/src/asyncResolveFallback.ts +++ b/packages/utils/src/asyncResolveFallback.ts @@ -20,7 +20,7 @@ export const syncResolve = ( what: string, importer: string, stack: string[] -) => { +): string => { const where = [importer, ...stack].map((p) => path.dirname(p)); const resolved = safeResolve(what, where); if (!(resolved instanceof Error)) { @@ -41,7 +41,11 @@ export const syncResolve = ( throw resolved; }; -const asyncResolve = (what: string, importer: string, stack: string[]) => { +const asyncResolve = ( + what: string, + importer: string, + stack: string[] +): Promise => { const where = [importer, ...stack].map((p) => path.dirname(p)); const resolved = safeResolve(what, where); if (!(resolved instanceof Error)) { diff --git a/packages/utils/src/collectExportsAndImports.ts b/packages/utils/src/collectExportsAndImports.ts index 6e4439d4b..0c75190bd 100644 --- a/packages/utils/src/collectExportsAndImports.ts +++ b/packages/utils/src/collectExportsAndImports.ts @@ -317,15 +317,30 @@ function collectFromDynamicImport(path: NodePath, state: IState): void { } } -function getImportTypeByInteropFunction( +function getImportExportTypeByInteropFunction( path: NodePath -): '*' | 'default' | undefined { +): 'import:*' | 're-export:*' | 'default' | undefined { const callee = path.get('callee'); - if (!callee.isIdentifier()) { + let name: string | undefined; + if (callee.isIdentifier()) { + name = callee.node.name; + } + + if (callee.isMemberExpression()) { + const property = callee.get('property'); + if (property.isIdentifier()) { + name = property.node.name; + } + } + + if (name === undefined) { return undefined; } - const { name } = callee.node; + if (name.startsWith('__exportStar')) { + return 're-export:*'; + } + if ( name.startsWith('_interopRequireDefault') || name.startsWith('__importDefault') @@ -338,7 +353,7 @@ function getImportTypeByInteropFunction( name.startsWith('__importStar') || name.startsWith('__toESM') ) { - return '*'; + return 'import:*'; } if ( @@ -346,7 +361,7 @@ function getImportTypeByInteropFunction( name.startsWith('__objRest') || name.startsWith('_objectDestructuringEmpty') ) { - return '*'; + return 'import:*'; } return undefined; @@ -374,7 +389,7 @@ function collectFromRequire(path: NodePath, state: IState): void { if (container.isCallExpression() && key === 0) { // It may be transpiled import such as // `var _atomic = _interopRequireDefault(require("@linaria/atomic"));` - const imported = getImportTypeByInteropFunction(container); + const imported = getImportExportTypeByInteropFunction(container); if (!imported) { // It's not a transpiled import. // TODO: Can we guess that it's a namespace import? @@ -386,6 +401,17 @@ function collectFromRequire(path: NodePath, state: IState): void { return; } + if (imported === 're-export:*') { + state.reexports.push({ + exported: '*', + imported: '*', + local: path, + source, + }); + + return; + } + let { parentPath: variableDeclarator } = container; if (variableDeclarator.isCallExpression()) { if (variableDeclarator.get('callee').isIdentifier({ name: '_extends' })) { @@ -413,9 +439,9 @@ function collectFromRequire(path: NodePath, state: IState): void { return; } - if (imported === '*') { + if (imported === 'import:*') { const unfolded = unfoldNamespaceImport({ - imported, + imported: '*', local: id, source, type: 'cjs',