&ai Add partner record handling - #97
Conversation
| const { NyplSourceMapper, config } = require('@nypl/node-utils') | ||
|
|
||
| let nyplSourceMapperInstance = null | ||
| const getNyplSourceMapper = async () => { |
There was a problem hiding this comment.
this is totally fine, but the idea of the NyplSourceMapper is to load the instance in some initializer method at a high level in the app. Then the NyplSourceMapper.instance() is available as a synchronous method without having to deal with building your own singleton method. The module itself is already handling that.
| nyplSourceMapperInstance = NyplSourceMapper.instance() | ||
| } | ||
| return nyplSourceMapperInstance | ||
| } |
There was a problem hiding this comment.
Looks like we may have some issues in the singleton methods in nypl-source-mapper (NyplSourceMapper.loadInstance() creates a new instance every time) obligating you to write your own singleton handling here. That's fine for now, but we really ought to fix it in node-utils.
Also, I don't think you're using node-utils config, although you're requiring it and running config.loadConfig. Seems like config access in this app is already handled through a straight dotenv.config call; Loading config as you're doing here doesn't do anything. It would be cool to use node-utils config, but that seems maybe beyond the scope.
3 participants
/record=path