refactor: added pacote to resolve template name instead of arborist workaround #1287
Conversation
ShashwatPS
requested review from
derberg,
magicmatatjahu,
jonaslagoni and
asyncapi-bot-eve
as code owners
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
| if (!addResult) throw new Error('Unable to resolve the name of the added package. It was most probably not added to node_modules successfully'); | ||
|
|
||
| const packageName = addResult[0].name; | ||
| const manifest = await pacote.manifest(this.templateName); |
There was a problem hiding this comment.
Here how errors are handled? If they are not pls. add conditions for that
There was a problem hiding this comment.
Pls. remove empty file if its not required
There was a problem hiding this comment.
Remove this empty file also
| const generatorReactSdk = require('@asyncapi/generator-react-sdk'); | ||
| const jsxRuntime = require('react/cjs/react-jsx-runtime.production.min'); | ||
| var generatorReactSdk = require('@asyncapi/generator-react-sdk'); | ||
| var jsxRuntime = require('C:/Users/shash/Desktop/generator/node_modules/react/cjs/react-jsx-runtime.production.min.js'); |
There was a problem hiding this comment.
Here the file path is compatible to windows OS, if someone is using Linux or Mac. Won't it fail to execute?
|
@ShashwatPS do you plan to followup with initial review from @DhairyaMajmudar ? |
|
no more reaction from contributor closing @ShashwatPS feel free to open a new PR if you want to continue. Just make sure to take into account feedback from @DhairyaMajmudar |
Description
pacotedependency.arbortismock and added a new one forpacote.generator.jsas suggested.Related issue(s)