Skip to content

Commit 0f76198

Browse files
nklaymannklayman
committed
fix(native modules): don't add node_modules to globalPaths
1 parent 94e10b5 commit 0f76198

File tree

6 files changed

+15
-173
lines changed

6 files changed

+15
-173
lines changed

‎__tests__/checkLogs.helper.js‎

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,6 @@ module.exports = async ({ client, projectPath, projectName, mode }) => {
3636
isBuild ? outputPath : projectPath('public')
3737
)
3838

39-
let modulePaths = logs
40-
// Find modulePath log
41-
.find(v => v.message.indexOf('modulePaths=') !== -1)
42-
// Get just the value
43-
.message.split('=')[1]
44-
// Remove last quote
45-
modulePaths = modulePaths.replace(/"$/, '')
46-
// Parse modulePaths array
47-
modulePaths = modulePaths.split(',')
48-
// Normalize paths
49-
modulePaths = modulePaths.map(p => path.normalize(p))
50-
// module.paths should include path to project's node_modules unless in build
51-
if (isBuild) {
52-
expect(modulePaths).not.toContain(
53-
path.join(__dirname, 'projects', projectName, 'node_modules')
54-
)
55-
} else {
56-
expect(modulePaths).toContain(
57-
path.join(__dirname, 'projects', projectName, 'node_modules')
58-
)
59-
}
60-
6139
let vuePath = logs
6240
// Find vuePath log
6341
.find(v => v.message.indexOf('vuePath=') !== -1)
@@ -98,26 +76,6 @@ module.exports = async ({ client, projectPath, projectName, mode }) => {
9876
isBuild ? outputPath : projectPath('public')
9977
)
10078

101-
let modulePaths = logs
102-
// Find modulePath log
103-
.find(m => m.indexOf('modulePaths=') !== -1)
104-
// Get just the value
105-
.split('=')[1]
106-
// Parse modulePaths array
107-
modulePaths = modulePaths.split(',')
108-
// Normalize paths
109-
modulePaths = modulePaths.map(p => path.normalize(p.replace('"', '')))
110-
// module.paths should include path to project's node_modules unless in build
111-
if (isBuild) {
112-
expect(modulePaths).not.toContain(
113-
path.join(__dirname, 'projects', projectName, 'node_modules')
114-
)
115-
} else {
116-
expect(modulePaths).toContain(
117-
path.join(__dirname, 'projects', projectName, 'node_modules')
118-
)
119-
}
120-
12179
let mockExternalPath = logs
12280
// Find externalModulePath log
12381
.find(v => v.indexOf('mockExternalPath=') !== -1)

‎__tests__/createProject.helper.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ const createProject = (projectName, useTS, customPlugins = {}) =>
4040
`let mainWindow${useTS ? ': any' : ''}
4141
${useTS ? 'declare var __static: string' : ''}
4242
console.log('__static=' + __static)
43-
console.log('modulePaths=' + require('module').globalPaths)
4443
console.log('mockExternalPath=' + require.resolve('mockExternal'))`
4544
)
4645
// Have render process log __static and BASE_URL to console to make sure they are correct
@@ -50,7 +49,6 @@ const createProject = (projectName, useTS, customPlugins = {}) =>
5049
${useTS ? 'declare var __static: string' : ''}
5150
console.log('process.env.BASE_URL=' + process.env.BASE_URL)
5251
console.log('__static=' + __static )
53-
console.log('modulePaths=' + require('module').globalPaths)
5452
console.log('vuePath=' + require.resolve('vue'))
5553
console.log('mockExternalPath=' + require.resolve('mockExternal'))`
5654
)

‎__tests__/generator.spec.js‎

Lines changed: 7 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ describe('.gitignore', () => {
6262
'apiResolve_./.gitignore',
6363
expect.any(String)
6464
)
65-
// Only index and background should have been written
66-
expect(fs.writeFileSync).toHaveBeenCalledTimes(2)
65+
// Only background.js should have been written
66+
expect(fs.writeFileSync).toHaveBeenCalledTimes(1)
6767
})
6868

6969
test.each(['#Electron-builder output', '/dist_electron'])(
@@ -100,13 +100,8 @@ describe('index.html', () => {
100100
test.each([
101101
// None
102102
'',
103-
// Node_modules path
104-
` <% if (VUE_APP_NODE_MODULES_PATH !== "false") { %><script>require('module').globalPaths.push('<%= VUE_APP_NODE_MODULES_PATH %>')</script><% } %>\n`,
105103
// Base URL
106-
` <% if (BASE_URL === './') { %><base href="app://./" /><% } %>\n`,
107-
// Both
108-
` <% if (BASE_URL === './') { %><base href="app://./" /><% } %>
109-
<% if (VUE_APP_NODE_MODULES_PATH !== "false") { %><script>require('module').globalPaths.push('<%= VUE_APP_NODE_MODULES_PATH %>')</script><% } %>\n`
104+
` <% if (BASE_URL === './') { %><base href="app://./" /><% } %>\n`
110105
])('Only add missing tags to index.html', existing => {
111106
// Disable .gitignore modification
112107
fs.existsSync.mockReturnValueOnce(false)
@@ -117,7 +112,7 @@ describe('index.html', () => {
117112
return JSON.stringify({ scripts: {} })
118113
}
119114
// return mock content
120-
return `<head>\n${existing} </head>`
115+
return ` <head>\n${existing} </head>`
121116
})
122117

123118
// Run the generator with mock api
@@ -126,22 +121,9 @@ describe('index.html', () => {
126121
completionCb()
127122

128123
const index = fs.writeFileSync.mock.calls[0][1]
129-
// Opening and closing tags still exist
130-
expect(index.match(/^\s*?<head.*?>\s*?$/gm).length).toBe(1)
131-
expect(index.match(/^\s*?<\/head.*?>\s*?$/gm).length).toBe(1)
132-
// Each tag exists once
133-
expect(
134-
index.match(
135-
// Base URL
136-
/<% if \(BASE_URL === '\.\/'\) { %><base href="app:\/\/\.\/" \/><% } %>/g
137-
).length
138-
).toBe(1)
139-
expect(
140-
index.match(
141-
// Node_modules path
142-
/<% if \(VUE_APP_NODE_MODULES_PATH !== "false"\) { %><script>require\('module'\)\.globalPaths\.push\('<%= VUE_APP_NODE_MODULES_PATH %>'\)<\/script><% } %>/g
143-
).length
144-
).toBe(1)
124+
expect(index).toBe(` <head>
125+
<% if (BASE_URL === './') { %><base href="app://./" /><% } %>
126+
</head>`)
145127
})
146128
})
147129

@@ -191,43 +173,6 @@ describe('background.js', () => {
191173
background
192174
)
193175
})
194-
195-
test.each([false, true])(
196-
'Background has node module path added if it does not exist',
197-
usesTS => {
198-
const file = `src/background.${usesTS ? 'ts' : 'js'}`
199-
// Mock having typescript
200-
mockApi.hasPlugin.mockImplementationOnce(
201-
plugin => plugin === 'typescript' && usesTS
202-
)
203-
// Mock not having node path added
204-
fs.readFileSync.mockImplementation((path, encoding) => {
205-
// Check that utf8 encoding is set
206-
expect(encoding).toBe('utf8')
207-
if (path === 'apiResolve_./package.json') {
208-
return JSON.stringify({ scripts: {} })
209-
}
210-
// return mock content
211-
return `const isDevelopment = process.env.NODE_ENV !== 'production'
212-
existing_content`
213-
})
214-
// Mock existence of background file
215-
fs.existsSync.mockImplementation(path => path === `apiResolve_./${file}`)
216-
generator(mockApi)
217-
completionCb()
218-
expect(mockApi.render).not.toBeCalled()
219-
expect(fs.writeFileSync).toBeCalledWith(
220-
`apiResolve_./${file}`,
221-
`const isDevelopment = process.env.NODE_ENV !== 'production'
222-
if (isDevelopment) {
223-
// Don't load any native (external) modules until the following line is run:
224-
require('module').globalPaths.push(process.env.NODE_MODULES_PATH)
225-
}
226-
227-
existing_content`
228-
)
229-
}
230-
)
231176
})
232177

233178
describe('package.json', () => {

‎generator/index.js‎

Lines changed: 6 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,13 @@ module.exports = api => {
1010
api.render('./template')
1111
}
1212
api.onCreateComplete(() => {
13-
// Read existing index.html and .gitignore
13+
// Read existing index.html
1414
let index = fs.readFileSync(api.resolve('./public/index.html'), 'utf8')
15-
16-
// List of tags that need to be included in index.html
17-
const tags = [
18-
`<% if (BASE_URL === './') { %><base href="app://./" /><% } %>`,
19-
`<% if (VUE_APP_NODE_MODULES_PATH !== "false") { %><script>require('module').globalPaths.push('<%= VUE_APP_NODE_MODULES_PATH %>')</script><% } %>`
20-
]
21-
let elements = ' <head>'
22-
tags.forEach(tag => {
23-
if (index.indexOf(tag) === -1) {
24-
elements += `\n ${tag}`
25-
}
26-
})
27-
// Add extra elements inside <head> tag
28-
index = index.replace(/^\s*?<head.*?>\s*?$/m, elements)
15+
const tag = `<% if (BASE_URL === './') { %><base href="app://./" /><% } %>`
16+
if (index.indexOf(tag) === -1) {
17+
// Inject base tag if it doesn't exist
18+
index = index.replace(/^\s*?<head.*?>\s*?$/m, ` <head>\n ${tag}`)
19+
}
2920
// Write updated index.html
3021
fs.writeFileSync(api.resolve('./public/index.html'), index)
3122

@@ -39,45 +30,6 @@ module.exports = api => {
3930
}
4031
}
4132

42-
if (hasBackground) {
43-
// Find background file
44-
let background
45-
if (fs.existsSync(api.resolve('./src/background.js'))) {
46-
background = fs.readFileSync(api.resolve('./src/background.js'), 'utf8')
47-
} else if (fs.existsSync(api.resolve('./src/background.ts'))) {
48-
background = fs.readFileSync(api.resolve('./src/background.ts'), 'utf8')
49-
} else {
50-
// Exit if background file cannot be found
51-
return
52-
}
53-
54-
if (
55-
// Only change file if it is missing module path addition
56-
!background.match(
57-
/require\('module'\)\.globalPaths\.push\(process\.env\.NODE_MODULES_PATH\)/
58-
)
59-
) {
60-
// Remove old isDevelopment const
61-
background = background.replace(
62-
"const isDevelopment = process.env.NODE_ENV !== 'production'",
63-
''
64-
)
65-
// Add node_modules path and isDevelopment const
66-
background =
67-
`const isDevelopment = process.env.NODE_ENV !== 'production'
68-
if (isDevelopment) {
69-
// Don't load any native (external) modules until the following line is run:
70-
require('module').globalPaths.push(process.env.NODE_MODULES_PATH)
71-
}\n` + background
72-
73-
// Write new background
74-
fs.writeFileSync(
75-
api.resolve(`./src/background.${usesTS ? 'ts' : 'js'}`),
76-
background
77-
)
78-
}
79-
}
80-
8133
if (usesTS) {
8234
let background
8335
if (fs.existsSync(api.resolve('./src/background.js'))) {

‎generator/template/src/background.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@ import {
66
installVueDevtools
77
} from 'vue-cli-plugin-electron-builder/lib'
88
const isDevelopment = process.env.NODE_ENV !== 'production'
9-
if (isDevelopment) {
10-
// Don't load any native (external) modules until the following line is run:
11-
require('module').globalPaths.push(process.env.NODE_MODULES_PATH)
12-
}
139

1410
// Keep a global reference of the window object, if you don't, the window will
1511
// be closed automatically when the JavaScript object is garbage collected.

‎lib/webpackConfig.js‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,7 @@ async function chainWebpack (api, pluginOptions, config) {
1818
args[0].__static = '__dirname'
1919
return args
2020
})
21-
// Don't add node_modules path
22-
process.env.VUE_APP_NODE_MODULES_PATH = false
2321
} else if (process.env.NODE_ENV === 'development') {
24-
// Set node_modules path for externals
25-
process.env.VUE_APP_NODE_MODULES_PATH = api
26-
.resolve('./node_modules')
27-
.replace(/\\/g, '/')
2822
// Set __static to absolute path to public folder
2923
config.plugin('define').tap(args => {
3024
args[0].__static = JSON.stringify(api.resolve('./public'))
@@ -33,10 +27,9 @@ async function chainWebpack (api, pluginOptions, config) {
3327
}
3428
// Apply user config
3529
rendererProcessChain(config)
36-
} else {
37-
// Don't add node_modules path
38-
process.env.VUE_APP_NODE_MODULES_PATH = false
3930
}
31+
// Older generated files expect this
32+
process.env.VUE_APP_NODE_MODULES_PATH = false
4033
}
4134
// Find all the dependencies without a `main` property or with a `binary` property or set by user and add them as webpack externals
4235
function getExternals (api, pluginOptions) {

0 commit comments

Comments
 (0)