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

💣🐒 Reduce webpackers' config monkey-patching and use v5 default #3275

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 0 additions & 8 deletions .babelrc

This file was deleted.

4 changes: 2 additions & 2 deletions .circleci/asset_paths
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
Gemfile.lock
yarn.lock
.babelrc
.postcssrc.yml
app/javascript
app/assets
app/lib/pdf/images
babel.config.js
cdn
config/compass.rb
config/webpack
config/webpacker.yml
lib/developer_portal/app/assets
lib/developer_portal/app/views/developer_portal/css
lib/developer_portal/app/views/developer_portal/javascripts
Expand Down
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ jobs:
command: bundle exec rake assets:precompile:test
environment:
RAILS_GROUPS: assets
NODE_OPTIONS: --max-old-space-size=4096
- *save-assets-cache
- persist_to_workspace:
root: .
Expand Down
2 changes: 1 addition & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
}
},
"ignorePatterns": [
"jest.config.js",
"*.config.js",
"spec/javascripts/setupTests.ts",
"spec/javascripts/__mocks__/",
"app/javascript/src/Stats" // We better avoid touching this module, eslint may automatically fix some of the errors and break it.
Expand Down
13 changes: 8 additions & 5 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ config/amazon_s3.yml
db/sphinx
public/.com.apple*
*_flymake.rb
/node_modules/
/public/javascripts/cache/

/test/unit/stats/tmp/
Expand All @@ -125,9 +124,6 @@ vendor/cache
Gemfile.local

.powrc
/public/packs
/public/packs-test
/node_modules

/vendor/oracle/*.zip

Expand All @@ -141,4 +137,11 @@ Gemfile.local
.pnp.*
package-lock.json
*.orig
yarn-error.log

# Webpacker
/public/packs
/public/packs-test
/node_modules
/yarn-error.log
yarn-debug.log*
.yarn-integrity
3 changes: 0 additions & 3 deletions .postcssrc.yml

This file was deleted.

2 changes: 1 addition & 1 deletion app/javascript/src/Common/components/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const Select = <T extends IRecord>({
>
{isLoading && <Spinner className="pf-u-ml-md" size="md" />}
{/* Controllers expect an empty string for some operations (such as unsetting the default plan) */}
{item && <input name={name} type="hidden" value={item.id >= 0 ? item.id : ''} />}
{item && <input name={name} type="hidden" value={Number(item.id) >= 0 ? item.id : ''} />}
<PF4Select
aria-label={ariaLabel}
className={isClearable ? '' : 'pf-m-select__toggle-clear-hidden'}
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/buyer/stats_application.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
jlledom marked this conversation as resolved.
Show resolved Hide resolved
/** @jsx StatsUI.dom */
import { StatsUsageChart } from 'Stats/lib/usage_chart'
import { StatsUsageChartManager } from 'Stats/lib/usage_chart_manager'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/lib/applications_selector.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import { StatsUI } from 'Stats/lib/ui'
import $ from 'jquery'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/lib/applications_table.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import numeral from 'numeral'

Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/lib/csv_link.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import moment from 'moment-timezone'

Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/lib/menu.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */

import pluralize from 'pluralize'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/lib/methods_table.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import numeral from 'numeral'
import moment from 'moment-timezone'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/lib/metrics_selector.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import $ from 'jquery'
import numeral from 'numeral'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/provider/stats_application.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import { StatsUsageChart } from 'Stats/lib/usage_chart'
import { StatsUsageChartManager } from 'Stats/lib/usage_chart_manager'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/provider/stats_days_of_week.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */

import { PeriodRangeDate } from 'Stats/lib/state'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/provider/stats_hours_of_day.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */

import { PeriodRangeDate } from 'Stats/lib/state'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/provider/stats_top_apps.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import 'core-js/fn/array/find'
import moment from 'moment'
Expand Down
1 change: 1 addition & 0 deletions app/javascript/src/Stats/provider/stats_usage.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx StatsUI.dom */
import { StatsUsageChart } from 'Stats/lib/usage_chart'
import { PeriodRangeDate } from 'Stats/lib/state'
Expand Down
102 changes: 102 additions & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
module.exports = function(api) {
var validEnv = ['development', 'test', 'production']
var currentEnv = api.env()
var isDevelopmentEnv = api.env('development')
var isProductionEnv = api.env('production')
var isTestEnv = api.env('test')

if (!validEnv.includes(currentEnv)) {
throw new Error(
'Please specify a valid `NODE_ENV` or ' +
'`BABEL_ENV` environment variables. Valid values are "development", ' +
'"test", and "production". Instead, received: ' +
JSON.stringify(currentEnv) +
'.'
)
}

return {
presets: [
isTestEnv && [
'@babel/preset-env',
{
forceAllTransforms: true,
targets: {
node: 'current'
},
modules: 'commonjs'
},
'@babel/preset-react'
],
(isProductionEnv || isDevelopmentEnv) && [
'@babel/preset-env',
{
forceAllTransforms: true,
useBuiltIns: 'entry',
corejs: 3,
modules: false,
exclude: ['transform-typeof-symbol']
}
],
[
'@babel/preset-react',
{
runtime: 'automatic',
development: isDevelopmentEnv || isTestEnv,
useBuiltIns: true
}
],
['@babel/preset-typescript', { 'allExtensions': true, 'isTSX': true }]
].filter(Boolean),
plugins: [
'babel-plugin-macros',
'@babel/plugin-syntax-dynamic-import',
isTestEnv && 'babel-plugin-dynamic-import-node',
'@babel/plugin-transform-destructuring',
[
'@babel/plugin-proposal-class-properties',
{
loose: true
}
],
[
'@babel/plugin-proposal-object-rest-spread',
{
useBuiltIns: true
}
],
[
'@babel/plugin-proposal-private-methods',
{
loose: true
}
],
[
'@babel/plugin-proposal-private-property-in-object',
{
loose: true
}
],
[
'@babel/plugin-transform-runtime',
{
helpers: false,
regenerator: true,
corejs: false
}
],
[
'@babel/plugin-transform-regenerator',
{
async: false
}
],
isProductionEnv && [
'babel-plugin-transform-react-remove-prop-types',
{
removeImport: true
}
]
].filter(Boolean)
}
}
45 changes: 27 additions & 18 deletions config/webpack/development.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,32 @@
const BundleAnalyzerPlugin = require('webpack-bundle-analyzer').BundleAnalyzerPlugin;
const environment = require('./environment')
process.env.NODE_ENV = process.env.NODE_ENV || 'development'

// Add Webpack custom configs here
environment.loaders.append('eslint', {
test: /\.tsx?$/,
exclude: /(node_modules)/,
enforce: 'pre',
loader: 'eslint-loader',
options: {
eslintPath: 'eslint',
configFile: '.eslintrc'
}
})
const BundleAnalyzerPlugin = require('webpack-bundle-analyzer').BundleAnalyzerPlugin
const environment = require('./environment')
const ForkTsCheckerWebpackPlugin = require("fork-ts-checker-webpack-plugin")
const path = require('path')

// HACK: this removes transpilation errors in tests during development. tsconfig includes them so
// that VS Code can work with imports. Ideally we should have a specific config for VS Code but
// the plugin automatically picks `tsconfig.json` and doesn't support a custom filename.
const tsLoader = environment.loaders.get('ts')
tsLoader.options.reportFiles = [/!(spec\/javascripts)/]
// The default installation (by webpacker) only transpiles TS code using Babel. This enables type
// checking as part of the Webpack compilation process (i.e. fail the build if there are TS errors).
environment.plugins.append(
"ForkTsCheckerWebpackPlugin",
new ForkTsCheckerWebpackPlugin({
eslint: {
files: [
'./app/javascript/**/*.{ts,tsx}',
'./spec/javascripts/**/*.{ts,tsx}'
]
},
typescript: {
configFile: path.resolve(__dirname, "../../tsconfig.json"),
// TODO: this options is introduces in v8.0.0, it doesn't work yet.
// Ignore transpilation errors in specs during development. tsconfig includes them so
// that VS Code can work with imports. Ideally we should have a specific config for VS Code
// but the extension doesn't support custom config files.
reportFiles: ['app/javascript/**/*.{ts,tsx}'],
},
async: false,
})
)

environment.plugins.append('BundleAnalyzerPlugin',
new BundleAnalyzerPlugin()
Expand Down
40 changes: 14 additions & 26 deletions config/webpack/environment.js
Original file line number Diff line number Diff line change
@@ -1,44 +1,32 @@
const { environment } = require('@rails/webpacker')
const path = require('path')

// Add global webpack configs here

environment.loaders.delete('css')
// We don't use css modules, we can remove these loaders.
environment.loaders.delete('moduleCss')
environment.loaders.delete('sass')
environment.loaders.delete('moduleSass')

environment.loaders.append('ts', {
test: /.(ts|tsx)$/,
options: {},
loader: 'ts-loader'
// postCss is included by default by webpacker but it breaks our styles. Removing all plugins from
// its config file generates a lot of warning messages so better we remove the loader until we make
// it work. TODO: make it work.
;['css', 'sass'].forEach(key => {
const loader = environment.loaders.get(key)
loader.use = loader.use.filter(u => u.loader !== 'postcss-loader')
})

// Remove styles added automatically by @patternfly/react because it messes up our own styles.
// We import the necessary styles manually in our .scss files and that way it works.
environment.loaders.append('null', {
test: /\.css$/,
include: stylesheet => stylesheet.indexOf('@patternfly/react-styles/css/') > -1,
use: ['null-loader']
})

environment.loaders.append('style', {
test: /(\.css|\.scss|\.sass)$/,
use: [
{ loader: 'style-loader' },
{ loader: 'css-loader' },
{
loader: 'sass-loader',
options: {
modules: true,
localIdentName: '[name]---[local]---[hash:base64:5]'
}
}
]
})

// Quickstarts' guides are written in YAML for convenience (QuickStarts/templates), then this loader
// allow us to import them as JSON and pass them to the React component (QuickStartContainer).
environment.loaders.append('yaml', {
test: /\.ya?ml$/,
use: 'yaml-loader',
include: path.resolve(__dirname, '../../app/javascript'),
include: path.resolve(__dirname, '../../app/javascript/src/QuickStarts/templates'),
type: 'json'
})

Expand All @@ -54,13 +42,13 @@ environment.loaders.append('yaml', {
*/
const { output } = environment.config;
const oldPublicPath = output.publicPath
output.publicPath = '';
output.publicPath = ''

const fileLoader = environment.loaders.get('file');
Object.assign(fileLoader.use[0].options, {
publicPath: oldPublicPath,
postTransformPublicPath: (p) => `window.rails_asset_host + ${p}`
});
})

environment.config.merge({
optimization: {
Expand Down
19 changes: 15 additions & 4 deletions config/webpack/production.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
process.env.NODE_ENV = process.env.NODE_ENV || 'production'

const environment = require('./environment')
const ForkTsCheckerWebpackPlugin = require("fork-ts-checker-webpack-plugin")
const path = require('path')

// Add Webpack custom configs here

const tsLoader = environment.loaders.get('ts')
tsLoader.options.configFile = path.resolve(__dirname, '../../tsconfig.prod.json')
// The default installation (by webpacker) only transpiles TS code using Babel. This enables type
// checking as part of the Webpack compilation process (i.e. fail the build if there are TS errors).
environment.plugins.append(
"ForkTsCheckerWebpackPlugin",
new ForkTsCheckerWebpackPlugin({
typescript: {
configFile: path.resolve(__dirname, "../../tsconfig.prod.json"),
},
async: false,
})
)

return console.dir(environment.toWebpackConfig(), { depth: null })
module.exports = environment.toWebpackConfig()
Loading