Skip to content

Conversation

@dpraimeyuu
Copy link
Contributor

In this tiny PR:

  • dependencies were explicitly manifested in package.json - local development can be started
  • draft of documentation on the local setup was provided

@codesandbox
Copy link

codesandbox bot commented Mar 15, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link
Member

@henri-hulski henri-hulski left a comment

Choose a reason for hiding this comment

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

I think babel and webpack plug-ins should be under "devDependencies", as they are not used by the runtime but only for development.
You also should make sure that the deps are in sync with the deps in the root of the monorepo. You can check this by running npm run checkdeps from the root folder. After fix it manually as fixdeps doesn't really works as expected.
When you run npm install from the root folder of the monorepo all packages will be linked and build. Don't install dependencies in the package folders as this destroys the linking. All packages should be installed from the root folder during development.
BTW it seems there is a conflict. Make sure you start with the next branch.

@henri-hulski
Copy link
Member

BTW I have no idea why npm run start doesn't work for this package.

@dpraimeyuu
Copy link
Contributor Author

@henri-hulski I'll do how you advised - also documenting in the main readme (?) on how to proceed with bumping and maintaining dependencies.

@henri-hulski
Copy link
Member

Sure! I think that would help maintaining overmind.
Another detail, in the monorepo package.json we use exact dependency versions (without the ^) but in the packages we use normal versioning with ^.
Don’t know if this isn't a relic from the time when npm didn't have a lock file and maybe it's not necessary anymore, but for now repo-cooker checks for it.
For more information how repo-cooker works look at its Readme or ask me. 😊

Copy link
Member

@henri-hulski henri-hulski left a comment

Choose a reason for hiding this comment

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

Hi Damian! Could you take a look at the review and fix it? Then I will merge it.

"devtool"
],
"dependencies": {
"@babel/plugin-transform-runtime": "^7.24.0",
Copy link
Member

Choose a reason for hiding this comment

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

All webpack and babel related dependencies are devDependencies not dependencies as they're only needed for development, not when you install the npm package (npm install overmind-devtools).

"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-dom": "^18.2.0",
"react-icons": "^5.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

react-icons is ok (should be) in dependencies as it's used directly in the code.

@@ -1,3 +1,22 @@
# overmind-devtools
### Introduction
Copy link
Member

Choose a reason for hiding this comment

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

We should leave # overmind-devtools so that we know where we are and underneath use ## not ###. Otherwise I get linting errors.

Copy link
Member

Choose a reason for hiding this comment

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

Also seems like you need to rebase as there were some changes in package.json.
Btw I'm just starting to fix the start scripts. For overmin-devtools-client I was able to make the web-client work.
I also want to take a look why npm start doesn't work in this repo.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants