Skip to content

Commit e06d565

Browse files
gauravtiwarigauravtiwari
committed
Consitently use NODE_ENV and restructure npm package for clarity
1 parent f01102a commit e06d565

30 files changed

+374
-168
lines changed

‎lib/webpacker.rb

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ def instance
1919
end
2020

2121
require "webpacker/instance"
22+
require "webpacker/env"
2223
require "webpacker/configuration"
2324
require "webpacker/manifest"
2425
require "webpacker/compiler"

‎lib/webpacker/dev_server_runner.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def run
1414
private
1515
def load_config
1616
@config_file = File.join(@app_path, "config/webpacker.yml")
17-
dev_server = YAML.load_file(@config_file)[ENV["RAILS_ENV"]]["dev_server"]
17+
dev_server = YAML.load_file(@config_file)[ENV["NODE_ENV"]]["dev_server"]
1818

1919
@hostname = dev_server["host"]
2020
@port = dev_server["port"]

‎lib/webpacker/env.rb

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
class Webpacker::Env
2+
DEFAULT = "production".freeze
3+
4+
delegate :config_path, :logger, to: :@webpacker
5+
6+
def self.inquire(webpacker)
7+
new(webpacker).inquire
8+
end
9+
10+
def initialize(webpacker)
11+
@webpacker = webpacker
12+
end
13+
14+
def inquire
15+
fallback_env_warning unless current
16+
(current || DEFAULT).inquiry
17+
end
18+
19+
private
20+
def current
21+
(ENV["NODE_ENV"] || Rails.env).presence_in(available_environments)
22+
end
23+
24+
def fallback_env_warning
25+
logger.info "NODE_ENV=#{ENV["NODE_ENV"]} and RAILS_ENV=#{Rails.env} environment is not defined in config/webpacker.yml, falling back to #{DEFAULT} environment"
26+
end
27+
28+
def available_environments
29+
if config_path.exist?
30+
YAML.load(config_path.read).keys
31+
else
32+
[].freeze
33+
end
34+
rescue Psych::SyntaxError => e
35+
raise "YAML syntax error occurred while parsing #{config_path}. " \
36+
"Please note that YAML must be consistently indented using spaces. Tabs are not allowed. " \
37+
"Error: #{e.message}"
38+
end
39+
end

‎lib/webpacker/instance.rb

+1-12
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ def initialize(root_path: Rails.root, config_path: Rails.root.join("config/webpa
88
end
99

1010
def env
11-
(ENV["NODE_ENV"].presence_in(available_environments) ||
12-
Rails.env.presence_in(available_environments) ||
13-
"production".freeze).inquiry
11+
@env ||= Webpacker::Env.inquire self
1412
end
1513

1614
def config
@@ -32,13 +30,4 @@ def manifest
3230
def commands
3331
@commands ||= Webpacker::Commands.new self
3432
end
35-
36-
private
37-
def available_environments
38-
if config_path.exist?
39-
YAML.load(config_path.read).keys
40-
else
41-
[].freeze
42-
end
43-
end
4433
end

‎package/__tests__/config.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
/* global test expect, describe */
22

3-
const chdirApp = () => process.chdir('test/test_app')
4-
const chdirCwd = () => process.chdir(process.cwd())
5-
chdirApp()
3+
const { chdirTestApp, chdirCwd } = require('../utils/helpers')
4+
5+
chdirTestApp()
66

77
const config = require('../config')
88

9-
describe('Webpacker.yml config', () => {
9+
describe('Config', () => {
1010
afterAll(chdirCwd)
1111

1212
test('should return extensions as listed in app config', () => {

‎package/__tests__/dev_server.js

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/* global test expect, describe */
2+
3+
const { chdirTestApp, chdirCwd } = require('../utils/helpers')
4+
5+
chdirTestApp()
6+
7+
describe('DevServer', () => {
8+
beforeEach(() => jest.resetModules())
9+
afterAll(chdirCwd)
10+
11+
test('with NODE_ENV set to development', () => {
12+
process.env.NODE_ENV = 'development'
13+
process.env.WEBPACKER_DEV_SERVER_HOST = '0.0.0.0'
14+
process.env.WEBPACKER_DEV_SERVER_PORT = 5000
15+
16+
const devServer = require('../dev_server')
17+
expect(devServer).toBeDefined()
18+
expect(devServer.host).toEqual('0.0.0.0')
19+
expect(devServer.port).toEqual('5000')
20+
})
21+
22+
test('with NODE_ENV set to production', () => {
23+
process.env.NODE_ENV = 'production'
24+
expect(require('../dev_server')).toEqual({})
25+
})
26+
})

‎package/__tests__/env.js

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/* global test expect, describe */
2+
3+
const { chdirTestApp, chdirCwd } = require('../utils/helpers')
4+
5+
chdirTestApp()
6+
7+
describe('Env', () => {
8+
beforeEach(() => jest.resetModules())
9+
afterAll(chdirCwd)
10+
11+
test('with NODE_ENV set to development', () => {
12+
process.env.NODE_ENV = 'development'
13+
expect(require('../env')).toEqual('development')
14+
})
15+
16+
test('with undefined NODE_ENV and RAILS_ENV set to development', () => {
17+
delete process.env.NODE_ENV
18+
process.env.RAILS_ENV = 'development'
19+
expect(require('../env')).toEqual('development')
20+
})
21+
22+
test('with a non-standard environment', () => {
23+
process.env.NODE_ENV = 'foo'
24+
process.env.RAILS_ENV = 'foo'
25+
expect(require('../env')).toEqual('production')
26+
delete process.env.RAILS_ENV
27+
})
28+
})

‎package/__tests__/index.js

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/* global test expect, describe */
2+
3+
const { chdirTestApp, chdirCwd } = require('../utils/helpers')
4+
5+
chdirTestApp()
6+
7+
describe('Webpacker', () => {
8+
beforeEach(() => jest.resetModules())
9+
afterAll(chdirCwd)
10+
11+
test('with NODE_ENV set to development', () => {
12+
process.env.NODE_ENV = 'development'
13+
const { environment } = require('../index')
14+
expect(environment.toWebpackConfig()).toMatchObject({
15+
devServer: {
16+
host: 'localhost',
17+
port: 3035
18+
}
19+
})
20+
})
21+
22+
test('with a non-standard env', () => {
23+
process.env.NODE_ENV = 'staging'
24+
process.env.RAILS_ENV = 'staging'
25+
const { environment } = require('../index')
26+
expect(environment.toWebpackConfig()).toMatchObject({
27+
devtool: 'nosources-source-map',
28+
stats: 'normal'
29+
})
30+
})
31+
})

‎package/config.js

+14-30
Original file line numberDiff line numberDiff line change
@@ -3,41 +3,25 @@ const { safeLoad } = require('js-yaml')
33
const { readFileSync } = require('fs')
44
const deepMerge = require('./utils/deep_merge')
55
const { isArray } = require('./utils/helpers')
6+
const env = require('./env')
67

7-
const defaultFilePath = require.resolve('../lib/install/config/webpacker.yml')
8-
const filePath = resolve('config', 'webpacker.yml')
8+
const defaultConfigPath = require.resolve('../lib/install/config/webpacker.yml')
9+
const configPath = resolve('config', 'webpacker.yml')
910

10-
const environment = process.env.NODE_ENV || 'development'
11-
const defaultConfig = safeLoad(readFileSync(defaultFilePath), 'utf8')[environment]
12-
const appConfig = safeLoad(readFileSync(filePath), 'utf8')[environment]
11+
const getConfig = () => {
12+
const defaults = safeLoad(readFileSync(defaultConfigPath), 'utf8')[env]
13+
const app = safeLoad(readFileSync(configPath), 'utf8')[env]
1314

14-
if (isArray(appConfig.extensions) && appConfig.extensions.length) {
15-
delete defaultConfig.extensions
16-
} else {
17-
/* eslint no-console: 0 */
18-
console.warn('No extensions specified in webpacker.yml, using default extensions\n')
19-
}
20-
21-
const config = deepMerge(defaultConfig, appConfig)
15+
if (isArray(app.extensions) && app.extensions.length) {
16+
delete defaults.extensions
17+
}
2218

23-
const isBoolean = str => /^true/.test(str) || /^false/.test(str)
19+
const config = deepMerge(defaults, app)
2420

25-
const fetch = key =>
26-
(isBoolean(process.env[key]) ? JSON.parse(process.env[key]) : process.env[key])
21+
config.outputPath = resolve('public', config.public_output_path)
22+
config.publicPath = `/${config.public_output_path}/`.replace(/([^:]\/)\/+/g, '$1')
2723

28-
const devServer = (key) => {
29-
const envValue = fetch(`WEBPACKER_DEV_SERVER_${key.toUpperCase().replace(/_/g, '')}`)
30-
if (typeof envValue === 'undefined' || envValue === null) return config.dev_server[key]
31-
return envValue
24+
return config
3225
}
3326

34-
if (config.dev_server) {
35-
Object.keys(config.dev_server).forEach((key) => {
36-
config.dev_server[key] = devServer(key)
37-
})
38-
}
39-
40-
config.outputPath = resolve('public', config.public_output_path)
41-
config.publicPath = `/${config.public_output_path}/`.replace(/([^:]\/)\/+/g, '$1')
42-
43-
module.exports = config
27+
module.exports = getConfig()

‎package/config_types/__tests__/config_list.js

-5
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ test('new', () => {
88
expect(list).toBeInstanceOf(Array)
99
})
1010

11-
test('set', () => {
12-
const list = new ConfigList()
13-
expect(list.set('key', 'value')).toEqual([{ key: 'key', value: 'value' }])
14-
})
15-
1611
test('get', () => {
1712
const list = new ConfigList()
1813
list.append('key', 'value')

‎package/config_types/config_list.js

-9
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,6 @@ class ConfigList extends Array {
1010
return this[index].value
1111
}
1212

13-
/**
14-
* @deprecated after the 3.0.2 release and will be removed in the next major release
15-
*/
16-
set(key, value) {
17-
/* eslint no-console: 0 */
18-
console.warn('set is deprecated! Use append instead')
19-
return this.append(key, value)
20-
}
21-
2213
append(key, value) {
2314
return this.add({ key, value })
2415
}

‎package/dev_server.js

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
const { isBoolean, isEmpty } = require('./utils/helpers')
2+
const config = require('./config')
3+
4+
const fetch = (key) => {
5+
const value = process.env[key]
6+
return isBoolean(value) ? JSON.parse(value) : value
7+
}
8+
9+
const devServer = () => {
10+
const devServerConfig = config.dev_server
11+
12+
if (devServerConfig) {
13+
Object.keys(devServerConfig).forEach((key) => {
14+
const envValue = fetch(`WEBPACKER_DEV_SERVER_${key.toUpperCase().replace(/_/g, '')}`)
15+
if (isEmpty(envValue)) return devServerConfig[key]
16+
devServerConfig[key] = envValue
17+
})
18+
}
19+
20+
return devServerConfig || {}
21+
}
22+
23+
module.exports = devServer()

‎package/env.js

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
const { resolve } = require('path')
2+
const { safeLoad } = require('js-yaml')
3+
const { readFileSync } = require('fs')
4+
5+
const configPath = resolve('config', 'webpacker.yml')
6+
const DEFAULT_ENV = 'production'
7+
8+
const env = () => {
9+
const nodeEnv = process.env.NODE_ENV
10+
const railsEnv = process.env.RAILS_ENV
11+
const config = safeLoad(readFileSync(configPath), 'utf8')
12+
const availableEnvironments = Object.keys(config).join('|')
13+
const regex = new RegExp(availableEnvironments, 'g')
14+
15+
if (nodeEnv && nodeEnv.match(regex)) return nodeEnv
16+
if (railsEnv && railsEnv.match(regex)) return railsEnv
17+
18+
/* eslint no-console: 0 */
19+
console.warn(`NODE_ENV=${nodeEnv} and RAILS_ENV=${railsEnv} environment is not defined in config/webpacker.yml, falling back to ${DEFAULT_ENV}`)
20+
return DEFAULT_ENV
21+
}
22+
23+
module.exports = env()

‎package/__tests__/environment.js ‎package/environments/__tests__/base.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
// environment.js expects to find config/webpacker.yml and resolved modules from
44
// the root of a Rails project
55

6-
const chdirApp = () => process.chdir('test/test_app')
7-
const chdirCwd = () => process.chdir(process.cwd())
8-
chdirApp()
6+
const { chdirTestApp, chdirCwd } = require('../../utils/helpers')
7+
8+
chdirTestApp()
99

1010
const { resolve } = require('path')
11-
const rules = require('../rules')
12-
const { ConfigList } = require('../config_types')
13-
const Environment = require('../environment')
11+
const rules = require('../../rules')
12+
const { ConfigList } = require('../../config_types')
13+
const Environment = require('../base')
1414

1515
describe('Environment', () => {
1616
afterAll(chdirCwd)

‎package/environment.js ‎package/environments/base.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ const ExtractTextPlugin = require('extract-text-webpack-plugin')
1212
const ManifestPlugin = require('webpack-manifest-plugin')
1313
const CaseSensitivePathsPlugin = require('case-sensitive-paths-webpack-plugin')
1414

15-
const { ConfigList, ConfigObject } = require('./config_types')
16-
const rules = require('./rules')
17-
const config = require('./config')
15+
const { ConfigList, ConfigObject } = require('../config_types')
16+
const rules = require('../rules')
17+
const config = require('../config')
1818

1919
const getLoaderList = () => {
2020
const result = new ConfigList()
@@ -85,7 +85,7 @@ const getBaseConfig = () =>
8585
}
8686
})
8787

88-
module.exports = class Environment {
88+
module.exports = class Base {
8989
constructor() {
9090
this.loaders = getLoaderList()
9191
this.plugins = getPluginList()

‎package/environments/development.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
const webpack = require('webpack')
2-
const Environment = require('../environment')
3-
const { dev_server: devServer, outputPath: contentBase, publicPath } = require('../config')
2+
const Base = require('./base')
3+
const devServer = require('../dev_server')
4+
const { outputPath: contentBase, publicPath } = require('../config')
45

5-
module.exports = class extends Environment {
6+
module.exports = class extends Base {
67
constructor() {
78
super()
89

0 commit comments

Comments
 (0)