Updates #2
Updates #2
Conversation
extension.js
Outdated
| @@ -169,12 +171,21 @@ function assertNextJSApp(componentPath) { | |||
| */ | |||
| async function executeCommand(commandInput, componentPath) { | |||
| const [command, ...args] = shellQuote.parse(commandInput); | |||
| const cp = child_process.spawn(command, args, { | |||
| const env = { ...process.env, PATH: `${process.env.PATH}:${componentPath}/node_modules/.bin` }; | |||
There was a problem hiding this comment.
I thought with an exec, that the pathing to the node_modules/.bin directory, but maybe I am misremembering. If this works, that's fine.
There was a problem hiding this comment.
I thought so too, but I believe that is strictly when npm is being invoked. Since the default build command is just next build we need to set the bin here.
Im considering changing the default to npm run build but there's some UX to consider there. I'll review tomorrow
There was a problem hiding this comment.
Okay so the reason why I can't have the default be npm run build, is that the ideal UX is that the user changes their build script to: harperdb-nextjs build. Thus, { "scripts": { "build": "harperdb-nextjs build" } } in their package.json. This enables the user to locally run npm run build and 🪄 . If the extension would only ever run npm run build, then this would create a infinite loop.
extension.js
Outdated
| @@ -169,12 +171,21 @@ function assertNextJSApp(componentPath) { | |||
| */ | |||
| async function executeCommand(commandInput, componentPath) { | |||
| const [command, ...args] = shellQuote.parse(commandInput); | |||
| const cp = child_process.spawn(command, args, { | |||
| const env = { ...process.env, PATH: `${process.env.PATH}:${componentPath}/node_modules/.bin` }; | |||
| let cp = child_process.spawn('which', [command], { cwd: componentPath, env, stdio: 'ignore' }); | |||
There was a problem hiding this comment.
If the command can't be found, doesn't the ENOENT error code unambiguously indicate that? Is it necessary to do a which first or can we just listen for errors on the main spawn call and translate ENOENT to the appropriate error message.
extension.js
Outdated
|
|
||
| const [exitCode] = await events.once(cp, 'exit'); | ||
| cp.on('error', (error) => { | ||
| throw error; |
There was a problem hiding this comment.
This is going to be an uncaught error, might be preferable to wrap this block in a Promise constructor so we could reject here.
package.json#main)node_modules/.binin the child_process path so thatnext buildworks correctlywhich