Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: cleanup and update dependencies #29

Merged
merged 2 commits into from
Oct 5, 2021
Merged

fix: cleanup and update dependencies #29

merged 2 commits into from
Oct 5, 2021

Conversation

smoya
Copy link
Member

@smoya smoya commented Oct 5, 2021

Description

This PR removes some dependencies that are not used. It also moves some dependencies from devDependencies to depencies when needed during runtime.

@smoya smoya requested a review from derberg October 5, 2021 11:18
derberg
derberg reviewed Oct 5, 2021
View reviewed changes
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments

package.json Outdated
"@asyncapi/modelina": "^0.24.0",
"puppeteer": "^5.2.1"
"@asyncapi/modelina": "^0.31.0",
"@babel/core": "^7.13.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need it, for sure not in dependencies and not even sure about devDependencies as it is needed for code transpilation and we don't do it here 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@babel/core is used for transpilation in the generator itself. We can remove it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, i thought exactly the same and I removed it but whenever I remove it, calling ag ... complains about missing dep.
Why?

Copy link
Member

@magicmatatjahu magicmatatjahu Oct 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you use the latest generator?

EDIT: I don't see any imports from @babel/core in the repo itself, so it should be bug on generator side. Please try generate another template like html-template and check if you will get this same error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not fail when running with the HTML template. This is the error it throws when generating with this template:

❯ ag -V
1.8.14

❯ ag ~/example.yaml . -o /tmp/wee
Something went wrong:
Error: Cannot find module '@babel/core'
Require stack:
- /Users/smoya/dev/go-template/node_modules/@rollup/plugin-babel/dist/index.js
- /Users/smoya/dev/go-template/node_modules/@asyncapi/generator-react-sdk/lib/transpiler/transpiler.js
- /Users/smoya/dev/go-template/node_modules/@asyncapi/generator-react-sdk/lib/transpiler/index.js
- /Users/smoya/dev/go-template/node_modules/@asyncapi/generator-react-sdk/lib/index.js
- /Users/smoya/dev/go-template/__transpiled/go.mod.js
- /usr/local/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/generator-react-sdk/lib/renderer/template.js
- /usr/local/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/generator-react-sdk/lib/renderer/index.js
- /usr/local/lib/node_modules/@asyncapi/generator/node_modules/@asyncapi/generator-react-sdk/lib/index.js
- /usr/local/lib/node_modules/@asyncapi/generator/lib/renderer/react.js
- /usr/local/lib/node_modules/@asyncapi/generator/lib/generator.js
- /usr/local/lib/node_modules/@asyncapi/generator/cli.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:902:15)
    at Function.Module._load (internal/modules/cjs/loader.js:746:27)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:93:18)
    at Object.<anonymous> (/Users/smoya/dev/go-template/node_modules/@rollup/plugin-babel/dist/index.js:5:13)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:93:18)
    at Object.<anonymous> (/Users/smoya/dev/go-template/node_modules/@asyncapi/generator-react-sdk/src/transpiler/transpiler.ts:4:1)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I had this same error, but please update deps to the latest versions like below:

    "@asyncapi/generator": "^1.8.14",
    "@asyncapi/parser": "^1.10.0",
    "@asyncapi/generator-filters": "^2.1.0",
    "@asyncapi/generator-hooks": "^0.1.0",
    "@asyncapi/generator-react-sdk": "^0.2.16",
    "@asyncapi/modelina": "^0.31.3"

and then everything should work :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is generator and parser for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and then everything should work :)

Do you mean without babel/core? It does not :/

"dependencies": {
    "@asyncapi/generator": "^1.8.14",
    "@asyncapi/parser": "^1.10.0",
    "@asyncapi/generator-filters": "^2.1.0",
    "@asyncapi/generator-hooks": "^0.1.0",
    "@asyncapi/generator-react-sdk": "^0.2.16",
    "@asyncapi/modelina": "^0.31.3"
  }
❯ ag ~/example.yaml . -o /tmp/wee
Something went wrong:
Error: Cannot find module '@babel/core'
...

what is generator and parser for?

+1 to this question. If I install babel/core and remove those two deps, it works.

Copy link
Member

@magicmatatjahu magicmatatjahu Oct 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only these deps (with fresh installation - removed package-lock, node_modules etc):

    "@asyncapi/generator-filters": "^2.0.0",
    "@asyncapi/generator-hooks": "^0.1.0",
    "@asyncapi/generator-react-sdk": "^0.2.11",
    "@asyncapi/modelina": "^0.31.0"

and it works 😅 so please remove parser and generator, because as I see it's not used anywhere in the code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked. I definitely know nothing about npm. I mean, nothing.

package.json Outdated Show resolved Hide resolved
@derberg
Copy link
Member

derberg commented Oct 5, 2021

@smoya please make it a fix: so we trigger a release workflow (in the end you do some dep cleanup), to release this and previous changes from @anandsunderraman

@smoya smoya changed the title chore: cleanup and update dependencies fix: cleanup and update dependencies Oct 5, 2021
@smoya
Copy link
Member Author

smoya commented Oct 5, 2021

@smoya please make it a fix: so we trigger a release workflow (in the end you do some dep cleanup), to release this and previous changes from @anandsunderraman

Just changed the PR title so once squashed it remains as fix: ...

@smoya smoya merged commit 8b58a61 into asyncapi:master Oct 5, 2021
@smoya smoya deleted the chore/updateDependencies branch October 5, 2021 19:51
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derberg derberg derberg approved these changes

@magicmatatjahu magicmatatjahu Awaiting requested review from magicmatatjahu

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@smoya @derberg @asyncapi-bot @magicmatatjahu