Monkeypatching source code of other packages is fragile

[gaearon]

As a maintainer of babel-preset-react-app I am not very happy about these lines:

react-firebase-starter/setup.js

Lines 32 to 64 in b917ae4

	//
	// Inject "babel-plugin-styled-components"
	// -----------------------------------------------------------------------------
	file = path.resolve('./node_modules/babel-preset-react-app/index.js');
	text = fs.readFileSync(file, 'utf8');
	if (!text.includes('babel-plugin-styled-components')) {
	if (text.includes('const plugins = [')) {
	text = text.replace(
	'const plugins = [',
	"const plugins = [\n require.resolve('babel-plugin-styled-components'),"); // prettier-ignore
	fs.writeFileSync(file, text, 'utf8');
	} else {
	throw new Error(`Failed to inject babel-plugin-styled-components in ${file}.`); // prettier-ignore
	}
	}
	//
	// Inject "babel-plugin-transform-export-extensions"
	// -----------------------------------------------------------------------------
	file = path.resolve('./node_modules/babel-preset-react-app/index.js');
	text = fs.readFileSync(file, 'utf8');
	if (!text.includes('babel-plugin-transform-export-extensions')) {
	if (text.includes('const plugins = [')) {
	text = text.replace(
	'const plugins = [',
	"const plugins = [\n require.resolve('babel-plugin-transform-export-extensions'),"); // prettier-ignore
	fs.writeFileSync(file, text, 'utf8');
	} else {
	throw new Error(`Failed to inject babel-plugin-transform-export-extensions in ${file}.`); // prettier-ignore
	}
	}

This is incredibly fragile, and is doing a disservice to all the people using this starter kit when we break this in the future.

It is quite possible that your users will be left with no good upgrade path. It is also very likely they are not experienced enough to figure out an alternative solution to this hack. In my opinion it’s irresponsible to rely on something like this without a clear disclaimer in the README that the project is built on a fragile foundation and can break with patch updates of the underlying tools.

I don’t have a great solution to this. Perhaps you could fork react-scripts or at least babel-preset-react-app? Even using Yarn resolutions to point to a fork would be less fragile than trying to monkeypatch the source depending on a variable name.

Thanks for consideration!

[ermik]

As the aforementioned "beginner", I am quite perplexed after understanding how flaky this workaround is. If not for @gaearon's feedback in the CRA repo it is very likely that I would've ended up using this kit to enable relay in one of the projects.

This also concerns this answer on StackOverflow which should at the very slightest bear an identical disclaimer.

Please clarify this across the board.

[koistya]

This is no longer relevant after migrating to React App SDK (#280).