MikeHopcroft / ShortOrder

A natural language conversational agent for ordering and organizing items from a catalog.
MIT License
13 stars 7 forks source link

Dependency on Node-specific functionality makes short-order unwebpackable #36

Open ryanvolum opened 5 years ago

ryanvolum commented 5 years ago

Webpacking short-order fails given its use of fs, dotenv and readline-sync, which are used specifically in a node runtime. If these packages are not core to short-order, please consider removing them so that short-order can run in browser.

See errors for more details:


Module not found: Error: Can't resolve 'child_process' in 'C:\git\Speech\WebSockets-Streaming\node_modules\readline-sync\lib'
 @ ./node_modules/readline-sync/lib/readline-sync.js 20:14-38
 @ ./node_modules/short-order/build/src/samples/repl_demo.js
 @ ./node_modules/short-order/build/src/samples/index.js
 @ ./node_modules/short-order/build/src/index.js
 @ ./lib/public/client.js

ERROR in ./node_modules/dotenv/lib/main.js
Module not found: Error: Can't resolve 'fs' in 'C:\git\Speech\WebSockets-Streaming\node_modules\dotenv\lib'
 @ ./node_modules/dotenv/lib/main.js 24:11-24
 @ ./node_modules/short-order/build/src/settings/settings.js
 @ ./node_modules/short-order/build/src/settings/index.js
 @ ./node_modules/short-order/build/src/index.js
 @ ./lib/public/client.js

ERROR in ./node_modules/readline-sync/lib/readline-sync.js
Module not found: Error: Can't resolve 'fs' in 'C:\git\Speech\WebSockets-Streaming\node_modules\readline-sync\lib'
 @ ./node_modules/readline-sync/lib/readline-sync.js 18:7-20
 @ ./node_modules/short-order/build/src/samples/repl_demo.js
 @ ./node_modules/short-order/build/src/samples/index.js
 @ ./node_modules/short-order/build/src/index.js
 @ ./lib/public/client.js

ERROR in ./node_modules/short-order/build/src/menu/menu.js
Module not found: Error: Can't resolve 'fs' in 'C:\git\Speech\WebSockets-Streaming\node_modules\short-order\build\src\menu'
 @ ./node_modules/short-order/build/src/menu/menu.js 3:11-24
 @ ./node_modules/short-order/build/src/menu/index.js
 @ ./node_modules/short-order/build/src/index.js
 @ ./lib/public/client.js

ERROR in ./node_modules/short-order/build/src/relevance_test/relevance_test.js
Module not found: Error: Can't resolve 'fs' in 'C:\git\Speech\WebSockets-Streaming\node_modules\short-order\build\src\relevance_test'
 @ ./node_modules/short-order/build/src/relevance_test/relevance_test.js 3:11-24
 @ ./node_modules/short-order/build/src/relevance_test/index.js
 @ ./node_modules/short-order/build/src/index.js
 @ ./lib/public/client.js

ERROR in ./node_modules/short-order/build/src/tokenizer/pattern_recognizer.js
Module not found: Error: Can't resolve 'fs' in 'C:\git\Speech\WebSockets-Streaming\node_modules\short-order\build\src\tokenizer'
 @ ./node_modules/short-order/build/src/tokenizer/pattern_recognizer.js 3:11-24
 @ ./node_modules/short-order/build/src/tokenizer/index.js
 @ ./node_modules/short-order/build/src/index.js
 @ ./lib/public/client.js```
ryanvolum commented 5 years ago

On further review, it appears that some of these dependencies only exist for the demos (which either aren't or shouldn't be part of the npm package). Consider making each sample its own project (with its own package.json/dependencies), and only include them in the repo

MikeHopcroft commented 5 years ago

I opened issue #40 regarding the factoring of samples code.

MikeHopcroft commented 5 years ago
MikeHopcroft commented 5 years ago

I may have unwittingly reintroduced fs when I moved the samples into src after factoring out token-flow.