diff --git a/code/core/src/common/utils/load-main-config.ts b/code/core/src/common/utils/load-main-config.ts index 579680e8da8f..07f48dda66de 100644 --- a/code/core/src/common/utils/load-main-config.ts +++ b/code/core/src/common/utils/load-main-config.ts @@ -29,9 +29,9 @@ export async function loadMainConfig({ if (!(e instanceof Error)) { throw e; } - if (e.message.includes('require is not defined')) { + if (e.message.includes('not defined in ES module scope')) { logger.info( - 'Loading main config failed, trying a temporary fix, Please ensure the main config is valid ESM' + 'Loading main config failed as the file does not seem to be valid ESM. Trying a temporary fix, please ensure the main config is valid ESM.' ); const comment = '// end of Storybook 10 migration assistant header, you can delete the above code'; diff --git a/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.test.ts b/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.test.ts index 05aadc126244..298768fb0e65 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.test.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.test.ts @@ -2,7 +2,7 @@ import { readFile, writeFile } from 'node:fs/promises'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { bannerComment } from '../helpers/mainConfigFile'; +import { bannerComment, containsDirnameUsage } from '../helpers/mainConfigFile'; import { fixFauxEsmRequire } from './fix-faux-esm-require'; vi.mock('node:fs/promises', async (importOriginal) => ({ @@ -76,7 +76,7 @@ describe('fix-faux-esm-require', () => { expect(result).toBeNull(); }); - it('should return true if file is ESM with require usage', async () => { + it('should return BannerConfig if file is ESM with require usage', async () => { const contentWithRequire = ` import { addons } from '@storybook/addon-essentials'; const config = require('./some-config'); @@ -91,7 +91,33 @@ describe('fix-faux-esm-require', () => { mainConfigPath: 'main.js', } as any); - expect(result).toBe(true); + expect(result).toEqual({ + hasRequireUsage: true, + hasUnderscoreDirname: false, + hasUnderscoreFilename: false, + }); + }); + + it('should return BannerConfig if file is ESM with __dirname usage but no definition', async () => { + const contentWithDirname = ` + import { addons } from '@storybook/addon-essentials'; + const configPath = path.join(__dirname, 'config.js'); + export default { + addons: ['@storybook/addon-essentials'], + }; + `; + + mockReadFile.mockResolvedValue(contentWithDirname); + + const result = await fixFauxEsmRequire.check({ + mainConfigPath: 'main.js', + } as any); + + expect(result).toEqual({ + hasRequireUsage: false, + hasUnderscoreDirname: true, + hasUnderscoreFilename: false, + }); }); it('should detect TypeScript config files', async () => { @@ -109,7 +135,96 @@ describe('fix-faux-esm-require', () => { mainConfigPath: 'main.ts', } as any); - expect(result).toBe(true); + expect(result).toEqual({ + hasRequireUsage: true, + hasUnderscoreDirname: false, + hasUnderscoreFilename: false, + }); + }); + }); + + describe('containsDirnameUsage', () => { + it('should detect __dirname in actual code', () => { + const content = ` + const path = require('path'); + const configPath = path.join(__dirname, 'config.js'); + `; + expect(containsDirnameUsage(content)).toBe(true); + }); + + it('should not detect __dirname in double-quoted strings', () => { + const content = ` + const message = "This is __dirname in a string"; + const another = "path/to/__dirname/file.js"; + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should not detect __dirname in single-quoted strings', () => { + const content = ` + const message = 'This is __dirname in a string'; + const another = 'path/to/__dirname/file.js'; + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should not detect __dirname in backtick strings', () => { + const content = ` + const message = \`This is __dirname in a string\`; + const another = \`path/to/__dirname/file.js\`; + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should not detect __dirname in single-line comments', () => { + const content = ` + // This is __dirname in a comment + const config = { addons: [] }; + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should not detect __dirname in multi-line comments', () => { + const content = ` + /* This is __dirname in a + multi-line comment */ + const config = { addons: [] }; + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should handle // inside strings correctly', () => { + const content = ` + const url = "https://example.com//path"; + const config = { addons: [] }; + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should detect __dirname when it appears after string with //', () => { + const content = ` + const url = "https://example.com//path"; + const configPath = path.join(__dirname, 'config.js'); + `; + expect(containsDirnameUsage(content)).toBe(true); + }); + + it('should handle escaped quotes in strings', () => { + const content = ` + const message = "This is \\"__dirname\\" in a string"; + const config = { addons: [] }; + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should handle mixed quote types', () => { + const content = ` + const double = "This is __dirname in double quotes"; + const single = 'This is __dirname in single quotes'; + const backtick = \`This is __dirname in backticks\`; + const actual = path.join(__dirname, 'config.js'); + `; + expect(containsDirnameUsage(content)).toBe(true); }); }); @@ -128,6 +243,11 @@ describe('fix-faux-esm-require', () => { await fixFauxEsmRequire.run({ dryRun: false, mainConfigPath: 'main.js', + result: { + hasRequireUsage: true, + hasUnderscoreDirname: false, + hasUnderscoreFilename: false, + }, } as any); expect(mockWriteFile).toHaveBeenCalledWith( @@ -136,13 +256,181 @@ describe('fix-faux-esm-require', () => { ); }); + it('should add __dirname support when needed', async () => { + const originalContent = ` + import { addons } from '@storybook/addon-essentials'; + const config = require('./some-config'); + const configPath = path.join(__dirname, 'config.js'); + export default { + addons: ['@storybook/addon-essentials'], + }; + `; + + mockReadFile.mockResolvedValue(originalContent); + + await fixFauxEsmRequire.run({ + dryRun: false, + mainConfigPath: 'main.js', + result: { + hasRequireUsage: true, + hasUnderscoreDirname: true, + hasUnderscoreFilename: false, + }, + } as any); + + const writtenContent = mockWriteFile.mock.calls[0][1]; + expect(writtenContent).toMatchInlineSnapshot(` + " + import { fileURLToPath } from "node:url"; + import { dirname } from "node:path"; + import { createRequire } from "node:module"; + import { addons } from '@storybook/addon-essentials'; + const __filename = fileURLToPath(import.meta.url); + const __dirname = dirname(__filename); + const require = createRequire(import.meta.url); + const config = require('./some-config'); + const configPath = path.join(__dirname, 'config.js'); + export default { + addons: ['@storybook/addon-essentials'], + }; + " + `); + }); + + it('should not add duplicate imports when they already exist', async () => { + const originalContent = ` + import { dirname } from "node:path"; + import { addons } from '@storybook/addon-essentials'; + const config = require('./some-config'); + const configPath = path.join(__dirname, 'config.js'); + export default { + addons: ['@storybook/addon-essentials'], + }; + `; + + mockReadFile.mockResolvedValue(originalContent); + + await fixFauxEsmRequire.run({ + dryRun: false, + mainConfigPath: 'main.js', + result: { + hasRequireUsage: true, + hasUnderscoreDirname: true, + hasUnderscoreFilename: false, + }, + } as any); + + const writtenContent = mockWriteFile.mock.calls[0][1]; + + expect(writtenContent).toMatchInlineSnapshot(` + " + import { fileURLToPath } from "node:url"; + import { createRequire } from "node:module"; + import { dirname } from "node:path"; + import { addons } from '@storybook/addon-essentials'; + const __filename = fileURLToPath(import.meta.url); + const __dirname = dirname(__filename); + const require = createRequire(import.meta.url); + const config = require('./some-config'); + const configPath = path.join(__dirname, 'config.js'); + export default { + addons: ['@storybook/addon-essentials'], + }; + " + `); + }); + it('should not write file in dry run mode', async () => { await fixFauxEsmRequire.run({ dryRun: true, mainConfigPath: 'main.js', + result: { + hasRequireUsage: true, + hasUnderscoreDirname: false, + hasUnderscoreFilename: false, + }, } as any); expect(mockWriteFile).not.toHaveBeenCalled(); }); + + it('should not add duplicate __filename/__dirname declarations when they already exist', async () => { + const originalContent = ` + import { addons } from '@storybook/addon-essentials'; + const __filename = 'existing-filename'; + const __dirname = 'existing-dirname'; + const config = require('./some-config'); + const configPath = path.join(__dirname, 'config.js'); + export default { + addons: ['@storybook/addon-essentials'], + }; + `; + + mockReadFile.mockResolvedValue(originalContent); + + await fixFauxEsmRequire.run({ + dryRun: false, + mainConfigPath: 'main.js', + result: { + hasRequireUsage: true, + hasUnderscoreDirname: true, + hasUnderscoreFilename: true, + }, + } as any); + + const writtenContent = mockWriteFile.mock.calls[0][1]; + expect(writtenContent).toMatchInlineSnapshot(` + " + import { fileURLToPath } from "node:url"; + import { dirname } from "node:path"; + import { createRequire } from "node:module"; + import { addons } from '@storybook/addon-essentials'; + const require = createRequire(import.meta.url); + const __filename = 'existing-filename'; + const __dirname = 'existing-dirname'; + const config = require('./some-config'); + const configPath = path.join(__dirname, 'config.js'); + export default { + addons: ['@storybook/addon-essentials'], + }; + " + `); + }); + + it('should not add duplicate require declaration when it already exists', async () => { + const originalContent = ` + import { addons } from '@storybook/addon-essentials'; + const require = createRequire(import.meta.url); + const config = require('./some-config'); + export default { + addons: ['@storybook/addon-essentials'], + }; + `; + + mockReadFile.mockResolvedValue(originalContent); + + await fixFauxEsmRequire.run({ + dryRun: false, + mainConfigPath: 'main.js', + result: { + hasRequireUsage: true, + hasUnderscoreDirname: false, + hasUnderscoreFilename: false, + }, + } as any); + + const writtenContent = mockWriteFile.mock.calls[0][1]; + expect(writtenContent).toMatchInlineSnapshot(` + " + import { createRequire } from "node:module"; + import { addons } from '@storybook/addon-essentials'; + const require = createRequire(import.meta.url); + const config = require('./some-config'); + export default { + addons: ['@storybook/addon-essentials'], + }; + " + `); + }); }); }); diff --git a/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.ts b/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.ts index c73d719b3aa5..04ffd79fa464 100644 --- a/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.ts +++ b/code/lib/cli-storybook/src/automigrate/fixes/fix-faux-esm-require.ts @@ -1,16 +1,69 @@ import { readFile, writeFile } from 'node:fs/promises'; +import { types as t } from 'storybook/internal/babel'; + import { dedent } from 'ts-dedent'; import { + type BannerConfig, bannerComment, + containsDirnameUsage, containsESMUsage, + containsFilenameUsage, containsRequireUsage, - getRequireBanner, hasRequireBanner, + updateMainConfig, } from '../helpers/mainConfigFile'; import type { Fix } from '../types'; +/** + * Checks if a variable declaration with the given identifier name exists in the program body. + * Covers var/let/const declarations and assignment patterns. + */ +function hasExistingDeclaration(program: t.Program, identifierName: string): boolean { + return program.body.some((node) => { + // Check variable declarations (const, let, var) + if (t.isVariableDeclaration(node)) { + return node.declarations.some((decl) => { + if (t.isVariableDeclarator(decl) && t.isIdentifier(decl.id)) { + return decl.id.name === identifierName; + } + return false; + }); + } + + // Check export named declarations with variable declarations + if ( + t.isExportNamedDeclaration(node) && + node.declaration && + t.isVariableDeclaration(node.declaration) + ) { + return node.declaration.declarations.some((decl) => { + if (t.isVariableDeclarator(decl) && t.isIdentifier(decl.id)) { + return decl.id.name === identifierName; + } + return false; + }); + } + + // Check function declarations + if (t.isFunctionDeclaration(node) && t.isIdentifier(node.id)) { + return node.id.name === identifierName; + } + + // Check export named declarations with function declarations + if ( + t.isExportNamedDeclaration(node) && + node.declaration && + t.isFunctionDeclaration(node.declaration) + ) { + return t.isIdentifier(node.declaration.id) && node.declaration.id.name === identifierName; + } + + return false; + }); +} + export const fixFauxEsmRequire = { id: 'fix-faux-esm-require', link: 'https://storybook.js.org/docs/faq#how-do-i-fix-module-resolution-in-special-environments', @@ -24,7 +77,6 @@ export const fixFauxEsmRequire = { const content = await readFile(mainConfigPath, 'utf-8'); const isESM = containsESMUsage(content); - const isWithRequire = containsRequireUsage(content); const isWithBanner = hasRequireBanner(content); // Check if the file is ESM format based on content @@ -37,29 +89,128 @@ export const fixFauxEsmRequire = { return null; } - // Check if the file contains require usage - if (!isWithRequire) { - return null; + // Analyze what compatibility features are needed + const hasRequireUsage = containsRequireUsage(content); + const hasUnderscoreFilename = containsFilenameUsage(content); + const hasUnderscoreDirname = containsDirnameUsage(content); + + // Check if any compatibility features are needed + if (hasRequireUsage || hasUnderscoreFilename || hasUnderscoreDirname) { + return { + hasRequireUsage, + hasUnderscoreDirname, + hasUnderscoreFilename, + }; } - return true; + return null; }, prompt() { - return dedent`Main config is ESM but uses 'require'. This will break in Storybook 10; Adding compatibility banner`; + return dedent`Main config is ESM but uses 'require' or '__dirname'. This will break in Storybook 10; Adding compatibility banner`; }, - async run({ dryRun, mainConfigPath }) { + async run({ dryRun, mainConfigPath, result }) { if (dryRun) { return; } - const content = await readFile(mainConfigPath, 'utf-8'); - const banner = getRequireBanner(); - const comment = bannerComment; + const { hasRequireUsage, hasUnderscoreDirname, hasUnderscoreFilename } = result; + + await updateMainConfig({ mainConfigPath, dryRun: !!dryRun }, (mainConfig) => { + // Only add imports for symbols that are actually used + if (hasRequireUsage) { + mainConfig.setImport(['createRequire'], 'node:module'); + } + if (hasUnderscoreDirname) { + mainConfig.setImport(['dirname'], 'node:path'); + } + if (hasUnderscoreFilename || hasUnderscoreDirname) { + mainConfig.setImport(['fileURLToPath'], 'node:url'); + } + + // Find the index after the last import declaration + const body = mainConfig._ast.program.body; + let lastImportIndex = -1; + for (let i = 0; i < body.length; i++) { + if (t.isImportDeclaration(body[i])) { + lastImportIndex = i; + } + } + const insertIndex = lastImportIndex + 1; + + // Check for existing declarations before inserting + const hasExistingFilename = hasExistingDeclaration(mainConfig._ast.program, '__filename'); + const hasExistingDirname = hasExistingDeclaration(mainConfig._ast.program, '__dirname'); + const hasExistingRequire = hasExistingDeclaration(mainConfig._ast.program, 'require'); + + // Add __filename and __dirname if used and not already declared + if (hasUnderscoreFilename || hasUnderscoreDirname) { + const declarationsToInsert: t.Statement[] = []; + let insertOffset = 0; - const newContent = [banner, comment, content].join('\n'); + // Add __filename declaration if needed and not already exists + // Note: __filename is always needed if __dirname is used, since __dirname depends on __filename + if ((hasUnderscoreFilename || hasUnderscoreDirname) && !hasExistingFilename) { + const filenameDeclaration = t.variableDeclaration('const', [ + t.variableDeclarator( + t.identifier('__filename'), + t.callExpression(t.identifier('fileURLToPath'), [ + t.memberExpression( + t.metaProperty(t.identifier('import'), t.identifier('meta')), + t.identifier('url') + ), + ]) + ), + ]); + declarationsToInsert.push(filenameDeclaration); + insertOffset++; + } + // Add __dirname declaration if needed and not already exists + if (hasUnderscoreDirname && !hasExistingDirname) { + const dirnameDeclaration = t.variableDeclaration('const', [ + t.variableDeclarator( + t.identifier('__dirname'), + t.callExpression(t.identifier('dirname'), [t.identifier('__filename')]) + ), + ]); + declarationsToInsert.push(dirnameDeclaration); + insertOffset++; + } + + // Insert declarations after imports + declarationsToInsert.forEach((declaration, index) => { + body.splice(insertIndex + index, 0, declaration); + }); + } + + // add require if used and not already declared + if (hasRequireUsage && !hasExistingRequire) { + const requireDeclaration = t.variableDeclaration('const', [ + t.variableDeclarator( + t.identifier('require'), + t.callExpression(t.identifier('createRequire'), [ + t.memberExpression( + t.metaProperty(t.identifier('import'), t.identifier('meta')), + t.identifier('url') + ), + ]) + ), + ]); + + // Calculate insert position: after imports and after any __filename/__dirname declarations that were added + const filenameAdded = + (hasUnderscoreFilename || hasUnderscoreDirname) && !hasExistingFilename; + const dirnameAdded = hasUnderscoreDirname && !hasExistingDirname; + const declarationsAdded = (filenameAdded ? 1 : 0) + (dirnameAdded ? 1 : 0); + const currentInsertIndex = insertIndex + declarationsAdded; + body.splice(currentInsertIndex, 0, requireDeclaration); + } + }); + + const content = await readFile(mainConfigPath, 'utf-8'); + const newContent = [bannerComment, content].join('\n'); await writeFile(mainConfigPath, newContent); }, -} satisfies Fix; +} satisfies Fix; diff --git a/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.test.ts b/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.test.ts index c61237b85035..4dfc15e19cd7 100644 --- a/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.test.ts +++ b/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.test.ts @@ -1,10 +1,13 @@ import { describe, expect, it } from 'vitest'; import { + containsDirnameUsage, + containsPatternUsage, getBuilderPackageName, getFrameworkPackageName, getRendererName, getRendererPackageNameFromFramework, + hasDirnameDefined, } from './mainConfigFile'; describe('getBuilderPackageName', () => { @@ -216,3 +219,114 @@ describe('getRendererPackageNameFromFramework', () => { expect(packageName).toBeNull(); }); }); + +describe('containsPatternUsage', () => { + it('should detect __dirname usage with hardcoded regex', () => { + const content = ` + const path = require('path'); + const configPath = path.join(__dirname, 'config.js'); + `; + expect(containsPatternUsage(content, /\b__dirname\b/)).toBe(true); + }); + + it('should not detect __dirname in comments', () => { + const content = ` + // This is __dirname in a comment + const path = require('path'); + `; + expect(containsPatternUsage(content, /\b__dirname\b/)).toBe(false); + }); + + it('should not detect __dirname in strings', () => { + const content = ` + const message = "This is __dirname in a string"; + const path = require('path'); + `; + expect(containsPatternUsage(content, /\b__dirname\b/)).toBe(false); + }); + + it('should return false when __dirname is not used', () => { + const content = ` + const path = require('path'); + const configPath = path.join(process.cwd(), 'config.js'); + `; + expect(containsPatternUsage(content, /\b__dirname\b/)).toBe(false); + }); + + it('should work with other regex patterns', () => { + const content = ` + const path = require('path'); + const configPath = path.join(__filename, 'config.js'); + `; + expect(containsPatternUsage(content, /\b__filename\b/)).toBe(true); + expect(containsPatternUsage(content, /\b__dirname\b/)).toBe(false); + }); +}); + +describe('containsDirnameUsage', () => { + it('should detect __dirname usage', () => { + const content = ` + const path = require('path'); + const configPath = path.join(__dirname, 'config.js'); + `; + expect(containsDirnameUsage(content)).toBe(true); + }); + + it('should not detect __dirname in comments', () => { + const content = ` + // This is __dirname in a comment + const path = require('path'); + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should not detect __dirname in strings', () => { + const content = ` + const message = "This is __dirname in a string"; + const path = require('path'); + `; + expect(containsDirnameUsage(content)).toBe(false); + }); + + it('should return false when __dirname is not used', () => { + const content = ` + const path = require('path'); + const configPath = path.join(process.cwd(), 'config.js'); + `; + expect(containsDirnameUsage(content)).toBe(false); + }); +}); + +describe('hasDirnameDefined', () => { + it('should detect const __dirname definition', () => { + const content = ` + const __dirname = dirname(__filename); + const path = require('path'); + `; + expect(hasDirnameDefined(content)).toBe(true); + }); + + it('should detect let __dirname definition', () => { + const content = ` + let __dirname = dirname(__filename); + const path = require('path'); + `; + expect(hasDirnameDefined(content)).toBe(true); + }); + + it('should detect var __dirname definition', () => { + const content = ` + var __dirname = dirname(__filename); + const path = require('path'); + `; + expect(hasDirnameDefined(content)).toBe(true); + }); + + it('should return false when __dirname is not defined', () => { + const content = ` + const path = require('path'); + const configPath = path.join(__dirname, 'config.js'); + `; + expect(hasDirnameDefined(content)).toBe(false); + }); +}); diff --git a/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.ts b/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.ts index ee3bd3e329c5..343cdccf1129 100644 --- a/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.ts +++ b/code/lib/cli-storybook/src/automigrate/helpers/mainConfigFile.ts @@ -255,22 +255,49 @@ export function containsRequireUsage(content: string): boolean { return requireCallRegex.test(content) || requireDotRegex.test(content); } -/** Check if the file already has the require banner */ -export const bannerComment = - '// end of Storybook 10 migration assistant header, You can delete the above code'; -export function hasRequireBanner(content: string): boolean { - return content.includes(bannerComment); +/** Check if the file content contains a pattern matching the given regex */ +export function containsPatternUsage(content: string, pattern: RegExp): boolean { + // Remove strings first, then comments + const stripStrings = (s: string) => s.replace(/(['"`])(?:\\.|(?!\1)[\s\S])*?\1/g, '""'); + const withoutStrings = stripStrings(content); + const withoutBlock = withoutStrings.replace(/\/\*[\s\S]*?\*\//g, ''); + const cleanContent = withoutBlock + .split('\n') + .map((line) => line.split('//')[0]) + .join('\n'); + + // Check for pattern usage in the cleaned content + return pattern.test(cleanContent); +} + +/** Check if the file content contains __dirname usage */ +export function containsDirnameUsage(content: string): boolean { + return containsPatternUsage(content, /\b__dirname\b/); +} + +export function containsFilenameUsage(content: string): boolean { + return containsPatternUsage(content, /\b__filename\b/); +} + +/** Check if __dirname is already defined in the file */ +export function hasDirnameDefined(content: string): boolean { + // Check if __dirname is already defined as a const/let/var + const dirnameDefinedRegex = /(?:const|let|var)\s+__dirname\s*=/; + return dirnameDefinedRegex.test(content); } -/** Generate the require compatibility banner */ -export function getRequireBanner(): string { - return dedent` - import { createRequire } from "node:module"; - import { dirname } from "node:path"; - import { fileURLToPath } from "node:url"; - - const __filename = fileURLToPath(import.meta.url); - const __dirname = dirname(__filename); - const require = createRequire(import.meta.url); - `; +/** Check if a specific import already exists in the file */ + +/** Configuration for what should be included in the compatibility banner */ +export interface BannerConfig { + hasRequireUsage: boolean; + hasUnderscoreDirname: boolean; + hasUnderscoreFilename: boolean; } + +export const bannerComment = + '// This file has been automatically migrated to valid ESM format by Storybook.'; + +export const hasRequireBanner = (content: string): boolean => { + return content.includes(bannerComment); +};