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

[No QA] [TS migration] Migrate 'Config' files to TypeScript #37718

Merged
merged 57 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
7b98ae0
update libraries to newer version with types, install types
JKobrynski Mar 4, 2024
630b8ae
migrate electronBuilder.config.js to TypeScript
JKobrynski Mar 4, 2024
1dbe373
migrate proxyConfig.js to TypeScript
JKobrynski Mar 4, 2024
c6a484f
migrate webpack.common.js to TypeScript
JKobrynski Mar 4, 2024
866211c
migrate webpack.desktop.js to TypeScript
JKobrynski Mar 4, 2024
847b4b7
start migrating webpack.dev.js to TypeScript
JKobrynski Mar 4, 2024
b29a9b3
add module declaration for preload-webpack-plugin
JKobrynski Mar 4, 2024
7ad015d
update scripts and documentation
JKobrynski Mar 4, 2024
ffc7b3d
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 4, 2024
21f6328
fix webpack.common.ts
JKobrynski Mar 4, 2024
5a6b1ee
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 5, 2024
59522b7
update imports in webpack.dev.ts
JKobrynski Mar 5, 2024
1819da7
update scripts in package.json, install webpack types
JKobrynski Mar 6, 2024
05b5c43
update file extensions in documentation
JKobrynski Mar 6, 2024
8bcb078
update tsconfig
JKobrynski Mar 6, 2024
56e5b17
update file extensions in documentation
JKobrynski Mar 6, 2024
b80efcc
update file extension in webpack.config.js
JKobrynski Mar 6, 2024
7649b02
migrate webpack.common.js to TypeScript
JKobrynski Mar 6, 2024
a0ba014
migrate webpack.desktop.js to TypeScript
JKobrynski Mar 6, 2024
512cf42
migrate webpack.dev.js to TypeScript
JKobrynski Mar 6, 2024
5ef518a
update file extensions in scripts
JKobrynski Mar 6, 2024
677243b
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 6, 2024
77a9b0d
start migrating CustomVersionFilePlugin to TypeScript
JKobrynski Mar 7, 2024
0141669
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 8, 2024
3c29359
Migrate CustomVersionFilePlugin to TypeScript
JKobrynski Mar 8, 2024
ca48f2c
revert electronBuilder.config.to to JavaScript
JKobrynski Mar 8, 2024
dafc6bb
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 12, 2024
21721cf
convert CustomVersionFilePlugin to esmodule
JKobrynski Mar 12, 2024
ec234b6
use import instead of require in webpack.dev.ts
JKobrynski Mar 12, 2024
a3e32ed
use import instead of require in CustomVersionFilePlugin
JKobrynski Mar 12, 2024
73406a6
replace require with import in webpack.common.ts, bump version of cle…
JKobrynski Mar 12, 2024
d11e749
replace require with import in webpack.desktop.ts
JKobrynski Mar 12, 2024
e09aadf
replace require with import in webpack.dev.ts
JKobrynski Mar 12, 2024
acedc7f
update file extension in readme
JKobrynski Mar 12, 2024
1a31c28
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 14, 2024
b2fb18f
remove esModuleInterop from tsconfig
JKobrynski Mar 15, 2024
7dc8d4d
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 18, 2024
0984f64
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 19, 2024
b05ea1a
apply minor improvements
JKobrynski Mar 19, 2024
433c321
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 20, 2024
ffdcc2b
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 20, 2024
55eb2d3
apply suggested changes
JKobrynski Mar 20, 2024
bbe50b6
update require to suit ESM in webpack.config.js
JKobrynski Mar 20, 2024
cc93841
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 21, 2024
e059dd1
apply suggested changes
JKobrynski Mar 21, 2024
0781659
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 22, 2024
b44cd0f
apply suggested changes;
JKobrynski Mar 25, 2024
170b3fe
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 25, 2024
6ec96dd
apply suggested changes to CustomVersionFilePlugin.ts
JKobrynski Mar 25, 2024
5bb52b4
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 25, 2024
d38ea29
apply suggested changes
JKobrynski Mar 26, 2024
bfa7795
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 26, 2024
39fe477
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 27, 2024
80796d3
apply suggested changes
JKobrynski Mar 27, 2024
0756e49
apply suggested changes to webpack config files
JKobrynski Mar 28, 2024
4c9144b
Merge branch 'main' into migrateConfigFilesToTypeScript
JKobrynski Mar 28, 2024
ed108c4
add additional if statements to webpack.common.ts
JKobrynski Mar 28, 2024
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
76 changes: 42 additions & 34 deletions .storybook/webpack.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,49 +31,57 @@ switch (process.env.ENV) {
}

const env = dotenv.config({path: path.resolve(__dirname, `../${envFile}`)});
const custom: CustomWebpackConfig = require('../config/webpack/webpack.common')({
const custom: CustomWebpackConfig = require('../config/webpack/webpack.common').default({
envFile,
});

const webpackConfig = ({config}: {config: Configuration}) => {
if (config.resolve && config.plugins && config.module) {
config.resolve.alias = {
'react-native-config': 'react-web-config',
'react-native$': 'react-native-web',
'@react-native-community/netinfo': path.resolve(__dirname, '../__mocks__/@react-native-community/netinfo.ts'),
'@react-navigation/native': path.resolve(__dirname, '../__mocks__/@react-navigation/native'),
...custom.resolve.alias,
};
if (!config.resolve) {
config.resolve = {};
}
if (!config.plugins) {
config.plugins = [];
}
if (!config.module) {
config.module = {};
}

// Necessary to overwrite the values in the existing DefinePlugin hardcoded to the Config staging values
const definePluginIndex = config.plugins.findIndex((plugin) => plugin instanceof DefinePlugin);
if (definePluginIndex !== -1 && config.plugins[definePluginIndex] instanceof DefinePlugin) {
const definePlugin = config.plugins[definePluginIndex] as DefinePlugin;
if (definePlugin.definitions) {
definePlugin.definitions.__REACT_WEB_CONFIG__ = JSON.stringify(env);
}
}
config.resolve.extensions = custom.resolve.extensions;
config.resolve.alias = {
'react-native-config': 'react-web-config',
'react-native$': 'react-native-web',
'@react-native-community/netinfo': path.resolve(__dirname, '../__mocks__/@react-native-community/netinfo.ts'),
'@react-navigation/native': path.resolve(__dirname, '../__mocks__/@react-navigation/native'),
...custom.resolve.alias,
};

const babelRulesIndex = custom.module.rules.findIndex((rule) => rule.loader === 'babel-loader');
const babelRule = custom.module.rules[babelRulesIndex];
if (babelRule) {
config.module.rules?.push(babelRule);
// Necessary to overwrite the values in the existing DefinePlugin hardcoded to the Config staging values
const definePluginIndex = config.plugins.findIndex((plugin) => plugin instanceof DefinePlugin);
if (definePluginIndex !== -1 && config.plugins[definePluginIndex] instanceof DefinePlugin) {
const definePlugin = config.plugins[definePluginIndex] as DefinePlugin;
if (definePlugin.definitions) {
definePlugin.definitions.__REACT_WEB_CONFIG__ = JSON.stringify(env);
}
}
config.resolve.extensions = custom.resolve.extensions;

const fileLoaderRule = config.module.rules?.find(
(rule): rule is RuleSetRule =>
typeof rule !== 'boolean' && typeof rule !== 'string' && typeof rule !== 'number' && !!rule?.test && rule.test instanceof RegExp && rule.test.test('.svg'),
);
if (fileLoaderRule) {
fileLoaderRule.exclude = /\.svg$/;
}
config.module.rules?.push({
test: /\.svg$/,
enforce: 'pre',
loader: require.resolve('@svgr/webpack'),
});
const babelRulesIndex = custom.module.rules.findIndex((rule) => rule.loader === 'babel-loader');
const babelRule = custom.module.rules[babelRulesIndex];
if (babelRule) {
config.module.rules?.push(babelRule);
}

const fileLoaderRule = config.module.rules?.find(
(rule): rule is RuleSetRule =>
typeof rule !== 'boolean' && typeof rule !== 'string' && typeof rule !== 'number' && !!rule?.test && rule.test instanceof RegExp && rule.test.test('.svg'),
);
if (fileLoaderRule) {
fileLoaderRule.exclude = /\.svg$/;
}
config.module.rules?.push({
test: /\.svg$/,
enforce: 'pre',
loader: require.resolve('@svgr/webpack'),
});

return config;
};
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ If you're using another operating system, you will need to ensure `mkcert` is in

## Running the web app 🕸
* To run the **development web app**: `npm run web`
* Changes applied to Javascript will be applied automatically via WebPack as configured in `webpack.dev.js`
* Changes applied to Javascript will be applied automatically via WebPack as configured in `webpack.dev.ts`

## Running the iOS app 📱
For an M1 Mac, read this [SO](https://stackoverflow.com/questions/64901180/how-to-run-cocoapods-on-apple-silicon-m1) for installing cocoapods.
Expand Down
9 changes: 8 additions & 1 deletion config/proxyConfig.js → config/proxyConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
* We only specify for staging URLs as API requests are sent to the production
* servers by default.
*/
module.exports = {
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
type ProxyConfig = {
STAGING: string;
STAGING_SECURE: string;
};

const proxyConfig: ProxyConfig = {
STAGING: '/staging/',
STAGING_SECURE: '/staging-secure/',
};

export default proxyConfig;
33 changes: 0 additions & 33 deletions config/webpack/CustomVersionFilePlugin.js

This file was deleted.

28 changes: 28 additions & 0 deletions config/webpack/CustomVersionFilePlugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import fs from 'fs';
import path from 'path';
import type {Compiler} from 'webpack';
import {version as APP_VERSION} from '../../package.json';

/**
* Simple webpack plugin that writes the app version (from package.json) and the webpack hash to './version.json'
*/
class CustomVersionFilePlugin {
apply(compiler: Compiler) {
compiler.hooks.done.tap(this.constructor.name, () => {
const versionPath = path.join(__dirname, '/../../dist/version.json');
fs.mkdir(path.dirname(versionPath), {recursive: true}, (directoryError) => {
if (directoryError) {
throw directoryError;
}
fs.writeFile(versionPath, JSON.stringify({version: APP_VERSION}), {encoding: 'utf8'}, (error) => {
if (!error) {
return;
}
throw error;
});
});
});
}
}

export default CustomVersionFilePlugin;
6 changes: 6 additions & 0 deletions config/webpack/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type Environment = {
file?: string;
platform?: 'web' | 'desktop';
};

export default Environment;
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
const path = require('path');
const fs = require('fs');
const {IgnorePlugin, DefinePlugin, ProvidePlugin, EnvironmentPlugin} = require('webpack');
const {CleanWebpackPlugin} = require('clean-webpack-plugin');
const HtmlWebpackPlugin = require('html-webpack-plugin');
const CopyPlugin = require('copy-webpack-plugin');
const dotenv = require('dotenv');
const {BundleAnalyzerPlugin} = require('webpack-bundle-analyzer');
import {CleanWebpackPlugin} from 'clean-webpack-plugin';
import CopyPlugin from 'copy-webpack-plugin';
import dotenv from 'dotenv';
import fs from 'fs';
import HtmlWebpackPlugin from 'html-webpack-plugin';
import path from 'path';
import type {Configuration} from 'webpack';
import {DefinePlugin, EnvironmentPlugin, IgnorePlugin, ProvidePlugin} from 'webpack';
import {BundleAnalyzerPlugin} from 'webpack-bundle-analyzer';
import CustomVersionFilePlugin from './CustomVersionFilePlugin';
import type Environment from './types';

// require is necessary, there are no types for this package and the declaration file can't be seen by the build process which causes an error.
const PreloadWebpackPlugin = require('@vue/preload-webpack-plugin');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💚

const CustomVersionFilePlugin = require('./CustomVersionFilePlugin');
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved

const includeModules = [
'react-native-animatable',
Expand All @@ -25,29 +29,25 @@ const includeModules = [
'expo-av',
].join('|');

const envToLogoSuffixMap = {
const environmentToLogoSuffixMap: Record<string, string> = {
production: '',
staging: '-stg',
dev: '-dev',
adhoc: '-adhoc',
};

function mapEnvToLogoSuffix(envFile) {
let env = envFile.split('.')[2];
if (typeof env === 'undefined') {
env = 'dev';
function mapEnvironmentToLogoSuffix(environmentFile: string): string {
let environment = environmentFile.split('.')[2];
if (typeof environment === 'undefined') {
environment = 'dev';
}
return envToLogoSuffixMap[env];
return environmentToLogoSuffixMap[environment];
}

/**
* Get a production grade config for web or desktop
* @param {Object} env
* @param {String} env.envFile path to the env file to be used
* @param {'web'|'desktop'} env.platform
* @returns {Configuration}
*/
const webpackConfig = ({envFile = '.env', platform = 'web'}) => ({
const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment): Configuration => ({
mode: 'production',
devtool: 'source-map',
entry: {
Expand All @@ -68,10 +68,10 @@ const webpackConfig = ({envFile = '.env', platform = 'web'}) => ({
new HtmlWebpackPlugin({
template: 'web/index.html',
filename: 'index.html',
splashLogo: fs.readFileSync(path.resolve(__dirname, `../../assets/images/new-expensify${mapEnvToLogoSuffix(envFile)}.svg`), 'utf-8'),
splashLogo: fs.readFileSync(path.resolve(__dirname, `../../assets/images/new-expensify${mapEnvironmentToLogoSuffix(file)}.svg`), 'utf-8'),
isWeb: platform === 'web',
isProduction: envFile === '.env.production',
isStaging: envFile === '.env.staging',
isProduction: file === '.env.production',
isStaging: file === '.env.staging',
}),
new PreloadWebpackPlugin({
rel: 'preload',
Expand Down Expand Up @@ -121,12 +121,14 @@ const webpackConfig = ({envFile = '.env', platform = 'web'}) => ({
...(platform === 'web' ? [new CustomVersionFilePlugin()] : []),
new DefinePlugin({
...(platform === 'desktop' ? {} : {process: {env: {}}}),
__REACT_WEB_CONFIG__: JSON.stringify(dotenv.config({path: envFile}).parsed),
// eslint-disable-next-line @typescript-eslint/naming-convention
__REACT_WEB_CONFIG__: JSON.stringify(dotenv.config({path: file}).parsed),

// React Native JavaScript environment requires the global __DEV__ variable to be accessible.
// react-native-render-html uses variable to log exclusively during development.
// See https://reactnative.dev/docs/javascript-environment
__DEV__: /staging|prod|adhoc/.test(envFile) === false,
// eslint-disable-next-line @typescript-eslint/naming-convention
__DEV__: /staging|prod|adhoc/.test(file) === false,
}),

// This allows us to interactively inspect JS bundle contents
Expand Down Expand Up @@ -203,21 +205,34 @@ const webpackConfig = ({envFile = '.env', platform = 'web'}) => ({
},
resolve: {
alias: {
// eslint-disable-next-line @typescript-eslint/naming-convention
'react-native-config': 'react-web-config',
// eslint-disable-next-line @typescript-eslint/naming-convention
'react-native$': 'react-native-web',
// eslint-disable-next-line @typescript-eslint/naming-convention
'react-native-sound': 'react-native-web-sound',
// Module alias for web & desktop
// https://webpack.js.org/configuration/resolve/#resolvealias
// eslint-disable-next-line @typescript-eslint/naming-convention
'@assets': path.resolve(__dirname, '../../assets'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@components': path.resolve(__dirname, '../../src/components/'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@hooks': path.resolve(__dirname, '../../src/hooks/'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@libs': path.resolve(__dirname, '../../src/libs/'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@navigation': path.resolve(__dirname, '../../src/libs/Navigation/'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@pages': path.resolve(__dirname, '../../src/pages/'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@styles': path.resolve(__dirname, '../../src/styles/'),
// This path is provide alias for files like `ONYXKEYS` and `CONST`.
// eslint-disable-next-line @typescript-eslint/naming-convention
'@src': path.resolve(__dirname, '../../src/'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@userActions': path.resolve(__dirname, '../../src/libs/actions/'),
// eslint-disable-next-line @typescript-eslint/naming-convention
'@desktop': path.resolve(__dirname, '../../desktop'),
},

Expand All @@ -242,6 +257,7 @@ const webpackConfig = ({envFile = '.env', platform = 'web'}) => ({
'.tsx',
],
fallback: {
// eslint-disable-next-line @typescript-eslint/naming-convention
'process/browser': require.resolve('process/browser'),
crypto: false,
},
Expand Down Expand Up @@ -275,4 +291,4 @@ const webpackConfig = ({envFile = '.env', platform = 'web'}) => ({
},
});

module.exports = webpackConfig;
export default getCommonConfiguration;
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,29 +1,28 @@
const path = require('path');
const webpack = require('webpack');
const _ = require('underscore');

const desktopDependencies = require('../../desktop/package.json').dependencies;
const getCommonConfig = require('./webpack.common');
import path from 'path';
import type {Configuration} from 'webpack';
import webpack from 'webpack';
// eslint-disable-next-line @dword-design/import-alias/prefer-alias, import/no-relative-packages -- alias imports don't work for webpack
import {dependencies as desktopDependencies} from '../../desktop/package.json';
import type Environment from './types';
import getCommonConfiguration from './webpack.common';

/**
* Desktop creates 2 configurations in parallel
* 1. electron-main - the core that serves the app content
* 2. web - the app content that would be rendered in electron
* Everything is placed in desktop/dist and ready for packaging
* @param {Object} env
* @returns {webpack.Configuration[]}
*/
module.exports = (env) => {
const rendererConfig = getCommonConfig({...env, platform: 'desktop'});
const getConfiguration = (environment: Environment): Configuration[] => {
const rendererConfig = getCommonConfiguration({...environment, platform: 'desktop'});
const outputPath = path.resolve(__dirname, '../../desktop/dist');

rendererConfig.name = 'renderer';
rendererConfig.output.path = path.join(outputPath, 'www');
(rendererConfig.output ??= {}).path = path.join(outputPath, 'www');

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this is the only place we use ??= ? I think this could be a bit more readable without that, but that's probably because I just learned this is valid syntax, so NAB

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you suggest we change it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, I don't feel very strongly, specially given the file it's in

// Expose react-native-config to desktop-main
const definePlugin = _.find(rendererConfig.plugins, (plugin) => plugin.constructor === webpack.DefinePlugin);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment above the line below it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's above, not 100% sure what the author had in mind though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈 I phrased that very poorly, but I think you got it. How does this line expose react-native-config to desktop-main? Isn't that done by plugins: [definePlugin],? this isn't a blocker

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "author" I meant the person that added the comment 😄

const definePlugin = rendererConfig.plugins?.find((plugin) => plugin?.constructor === webpack.DefinePlugin);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the ? in plugin?.constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, plugin might be null or undefined

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I quickly searched for the type of rendererConfig.plugins to try to figure this out and didn't find it, and I was assuming that if we can iterate over rendererConfig.plugins, then the items in it wouldn't be null or undefined

const mainProcessConfig = {
const mainProcessConfig: Configuration = {
mode: 'production',
name: 'desktop-main',
target: 'electron-main',
Expand All @@ -38,13 +37,15 @@ module.exports = (env) => {
},
resolve: rendererConfig.resolve,
plugins: [definePlugin],
externals: [..._.keys(desktopDependencies), 'fsevents'],
externals: [...Object.keys(desktopDependencies), 'fsevents'],
node: {
/**
* Disables webpack processing of __dirname and __filename, so it works like in node
* https://github.com/webpack/webpack/issues/2010
*/
// eslint-disable-next-line @typescript-eslint/naming-convention
__dirname: false,
// eslint-disable-next-line @typescript-eslint/naming-convention
__filename: false,
},
module: {
Expand All @@ -60,3 +61,5 @@ module.exports = (env) => {

return [mainProcessConfig, rendererConfig];
};

export default getConfiguration;
Loading
Loading