Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Config file instead of Environment Variables #2227

Merged
merged 28 commits into from
Dec 19, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2959334
Add cosmiconfig
AlbertoBrusa Dec 6, 2022
e1571dd
Allow use esbuild to be configured by a modular configuration file
AlbertoBrusa Dec 7, 2022
dac6289
Update comments
AlbertoBrusa Dec 7, 2022
7fb5370
Update config logic
AlbertoBrusa Dec 8, 2022
33742bf
Fix config
AlbertoBrusa Dec 9, 2022
cde234d
Change the default CDN to esm.sh
AlbertoBrusa Dec 9, 2022
dc77942
Update test snapshots for change to esm.sh
AlbertoBrusa Dec 9, 2022
cec53a1
More configurables
AlbertoBrusa Dec 12, 2022
f854b5e
https config
AlbertoBrusa Dec 12, 2022
249390f
Let JS files in react-scripts use config.ts using ts-node/register
AlbertoBrusa Dec 13, 2022
7e08f6a
Fix ts-node register
AlbertoBrusa Dec 13, 2022
b9b4f5b
More require ts-node register
AlbertoBrusa Dec 13, 2022
675b10f
Remove HOST and HTTPS config
AlbertoBrusa Dec 14, 2022
73fee89
Configurable PUBLIC_URL & GENERATE_SOURCEMAP
AlbertoBrusa Dec 15, 2022
b08bd37
Undo changes for PUBLIC_URL & GENERATE_SOURCEMAP config
AlbertoBrusa Dec 15, 2022
5054f28
2nd attempt re-implementing config for generate sourcemap and public url
AlbertoBrusa Dec 15, 2022
755a13f
Add changeset
AlbertoBrusa Dec 15, 2022
1a447a3
Documentation
AlbertoBrusa Dec 15, 2022
ee938f7
Update changelog
AlbertoBrusa Dec 15, 2022
7eead44
Update documentation
AlbertoBrusa Dec 15, 2022
5a94077
Update docs/releases/4.0.x.md
AlbertoBrusa Dec 16, 2022
142b7ef
Update documentation
AlbertoBrusa Dec 16, 2022
c085182
Merge branch 'config' of https://github.com/AlbertoBrusa/modular into…
AlbertoBrusa Dec 16, 2022
c8ef95c
Improved config function
AlbertoBrusa Dec 16, 2022
6175809
Update config function
AlbertoBrusa Dec 16, 2022
f73878e
Refactor config function
AlbertoBrusa Dec 16, 2022
7448a3b
Removed unnecessary type annotations
AlbertoBrusa Dec 16, 2022
f0e2e93
More cleaning up of Config()
AlbertoBrusa Dec 16, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions __fixtures__/test-config/.modular.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
useModularEsbuild: true,
};
1 change: 1 addition & 0 deletions packages/modular-scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"chalk": "4.1.2",
"change-case": "4.1.2",
"commander": "9.4.0",
"cosmiconfig": "^8.0.0",
"cross-spawn": "7.0.3",
"css-loader": "6.7.1",
"css-minimizer-webpack-plugin": "3.4.1",
Expand Down
51 changes: 51 additions & 0 deletions packages/modular-scripts/src/__tests__/utils/config.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import execa from 'execa';
import { copyFileSync } from 'fs';
import path from 'path';
import { createModularTestContext } from '../../test/utils';
import getModularRoot from '../../utils/getModularRoot';

const modularRoot = getModularRoot();
const configFixtures = path.join(modularRoot, '__fixtures__', 'test-config');

/**
* Run modular with provided arguments in specified directory
*/
function modular(
args: string,
cwd: string,
opts: Record<string, unknown> = {},
) {
return execa('yarnpkg', ['modular', ...args.split(' ')], {
cwd,
cleanup: true,
...opts,
});
}

// Temporary test context paths set by createTempModularRepoWithTemplate()
let tempModularRepo: string;

describe('A simple modular repo with a .modular.js config file', () => {
beforeEach(async () => {
tempModularRepo = createModularTestContext();
await modular('add test-app --unstable-type app', tempModularRepo);
copyFileSync(
path.join(configFixtures, '.modular.js'),
path.join(tempModularRepo, '.modular.js'),
);
});
it('builds using esbuild as specified in config file', async () => {
const result = await modular(`build test-app --verbose`, tempModularRepo);
expect(result.stdout).toContain('Building with esbuild');
expect(result.exitCode).toBe(0);
});
it('builds using webpack if the environment variable is provided as it overrides the config', async () => {
const result = await modular(`build test-app --verbose`, tempModularRepo, {
env: {
USE_MODULAR_WEBPACK: 'true',
},
});
expect(result.stdout).toContain('Building with Webpack');
expect(result.exitCode).toBe(0);
});
});
17 changes: 4 additions & 13 deletions packages/modular-scripts/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,13 @@ import {
} from './esbuildFileSizeReporter';
import { getDependencyInfo } from '../utils/getDependencyInfo';
import { isReactNewApi } from '../utils/isReactNewApi';
import { utilizeEsbuild } from '../utils/config';

async function buildStandalone(
target: string,
type: Extract<ModularType, 'app' | 'esm-view'>,
) {
// True if there's no preference set - or the preference is for webpack.
const useWebpack =
!process.env.USE_MODULAR_WEBPACK ||
process.env.USE_MODULAR_WEBPACK === 'true';

// True if the preferene IS set and the preference is esbuid.
const useEsbuild =
process.env.USE_MODULAR_ESBUILD &&
process.env.USE_MODULAR_ESBUILD === 'true';

// If you want to use webpack then we'll always use webpack. But if you've indicated
// you want esbuild - then we'll switch you to the new fancy world.
const isEsbuild = !useWebpack || useEsbuild;
const isEsbuild = await utilizeEsbuild();

// Setup Paths
const modularRoot = getModularRoot();
Expand Down Expand Up @@ -132,6 +121,7 @@ async function buildStandalone(
let cssEntryPoint: string | undefined;

if (isEsbuild) {
logger.debug('Building with esbuild');
const { default: buildEsbuildApp } = await import(
'../esbuild-scripts/build'
);
Expand All @@ -140,6 +130,7 @@ async function buildStandalone(
cssEntryPoint = getEntryPoint(paths, result, '.css');
assets = createEsbuildAssets(paths, result);
} else {
logger.debug('Building with Webpack');
// create-react-app doesn't support plain module outputs yet,
// so --preserve-modules has no effect here

Expand Down
13 changes: 2 additions & 11 deletions packages/modular-scripts/src/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import createEsbuildBrowserslistTarget from './utils/createEsbuildBrowserslistTa
import prompts from 'prompts';
import { getDependencyInfo } from './utils/getDependencyInfo';
import { isReactNewApi } from './utils/isReactNewApi';
import { utilizeEsbuild } from './utils/config';

async function start(packageName: string): Promise<void> {
let target = packageName;
Expand Down Expand Up @@ -61,16 +62,6 @@ async function start(packageName: string): Promise<void> {

await checkBrowsers(targetPath);

// True if there's no preference set - or the preference is for webpack.
const useWebpack =
!process.env.USE_MODULAR_WEBPACK ||
process.env.USE_MODULAR_WEBPACK === 'true';

// True if the preference IS set and the preference is esbuild.
const useEsbuild =
process.env.USE_MODULAR_ESBUILD &&
process.env.USE_MODULAR_ESBUILD === 'true';

// Retrieve dependency info for target to inform the build process
const {
importMap,
Expand Down Expand Up @@ -102,7 +93,7 @@ async function start(packageName: string): Promise<void> {

// If you want to use webpack then we'll always use webpack. But if you've indicated
// you want esbuild - then we'll switch you to the new fancy world.
if (!useWebpack || useEsbuild) {
if (await utilizeEsbuild()) {
const { default: startEsbuildApp } = await import(
'./esbuild-scripts/start'
);
Expand Down
4 changes: 4 additions & 0 deletions packages/modular-scripts/src/test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ export function createModularTestContext(): string {
'last 1 safari version',
],
},
dependencies: {
react: '^18.2.0',
'react-dom': '^18.2.0',
},
};

fs.writeJSONSync(path.join(tempModularRepo, 'package.json'), packageJson, {
Expand Down
91 changes: 91 additions & 0 deletions packages/modular-scripts/src/utils/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { cosmiconfig } from 'cosmiconfig';
import path from 'path';
import getModularRoot from './getModularRoot';
import * as logger from './logger';

// Where cosmiconfig can look for the configuration
const searchPlaces = [
'.modular.js',
'package.json',
`.modularrc`,
`.modularrc.json`,
`.modularrc.yaml`,
`.modularrc.yml`,
`.modularrc.js`,
`.modularrc.cjs`,
`modular.config.js`,
`modular.config.cjs`,
];

// Look for configuration file
const modularRoot = getModularRoot();
const explorer = cosmiconfig('modular', { searchPlaces });
const configuration = explorer.search(path.join(modularRoot, 'package.json'));

// Interface with all configurations
interface ConfigObject {
useModularEsbuild: boolean;
}

type ConfigObjectKey = keyof ConfigObject;

/**
* Get the configured value for a given configuration field.
* Rejects if no configuration file is present or the queired field is not present.
* @param configEntry Field containing the configuration variable to read
* @returns Value of configuration field queried if present
*/
export async function getConfiguration(
configEntry: ConfigObjectKey,
): Promise<boolean> {
const loadedConfiguration = await configuration;
// Handle no or empty configuration - debug log? don't think we should error should we?
if (loadedConfiguration) {
// Error if configuration doesn't match our interface?
const config = loadedConfiguration.config as ConfigObject;
const value = config[configEntry];
if (value) {
return value;
} else {
throw new Error(
AlbertoBrusa marked this conversation as resolved.
Show resolved Hide resolved
`No field ${configEntry.toString()} found in configuration file`,
);
}
} else {
throw new Error(
`Couldn't identify and load a valid modular configuration file`,
);
}
}

/**
* Reads env variables and configuration to understand if esbuild or Webpack should be used
* Webpack is used as default if no configuration is set or both are set to true
* Environment variables take precedence over config file
* @returns True if esbuild should be used or false if webpack should be used
*/
export async function utilizeEsbuild(): Promise<boolean> {
AlbertoBrusa marked this conversation as resolved.
Show resolved Hide resolved
if (
process.env.USE_MODULAR_WEBPACK === 'true' ||
process.env.USE_MODULAR_ESBUILD === 'false'
) {
return false;
}
if (
process.env.USE_MODULAR_ESBUILD === 'true' ||
process.env.USE_MODULAR_WEBPACK === 'false'
) {
return true;
} else {
return await getConfiguration('useModularEsbuild')
.then((result) => {
return result;
})
// Debug logging the errors as it's reasonable for users not to have provided a configuration,
// but it's useful to know while trying to figure out what's going wrong
.catch((err: Error) => {
logger.debug(err.message);
return false;
});
}
}
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4842,6 +4842,16 @@ cosmiconfig@^7.0.0, cosmiconfig@^7.0.1:
path-type "^4.0.0"
yaml "^1.10.0"

cosmiconfig@^8.0.0:
version "8.0.0"
resolved "https://registry.yarnpkg.com/cosmiconfig/-/cosmiconfig-8.0.0.tgz#e9feae014eab580f858f8a0288f38997a7bebe97"
integrity sha512-da1EafcpH6b/TD8vDRaWV7xFINlHlF6zKsGwS1TsuVJTZRkquaS5HTMq7uq6h31619QjbsYl21gVDOm32KM1vQ==
dependencies:
import-fresh "^3.2.1"
js-yaml "^4.1.0"
parse-json "^5.0.0"
path-type "^4.0.0"

create-jest-runner@^0.6.0:
version "0.6.0"
resolved "https://registry.yarnpkg.com/create-jest-runner/-/create-jest-runner-0.6.0.tgz#9ca6583d969acc15cdc21cd07d430945daf83de6"
Expand Down