-
Notifications
You must be signed in to change notification settings - Fork 459
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.
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
package.json updated #780
package.json updated #780
Conversation
package.json
Outdated
@@ -53,6 +50,7 @@ | |||
"engines": { | |||
"node": ">=4" | |||
}, | |||
"files":["@hoodie/admin", "@hoodie/client", "@hoodie/server"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "files"
key in the package.json
is not for names of dependencies, but for file names and paths that are part of the repository, please read the description at that’s not how you use files
. See the documentation at https://docs.npmjs.com/files/package.json#files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. see https://github.com/hoodiehq/hoodie-admin/blob/master/package.json#L14-L16. This means that when the package get published to the npm registry, only the contents of the dist/
folder get uploaded, everything else is ignored. Does that make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i did follow that. i am sure i misunderstood but i am still confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when you install hoodie
like it’s described in the quickstart guide and then open node_modules/hoodie
, you will see a ton of files in there, including the pandoc file which is 20mb of size. Hoodie doesn’t need that file to work, nor does it need e.g. the docs folder. The only file it needs are the JavaScript files that get executed when you run npm start
in your app. These are bin/
, cli/
and server/
. And then it also needs the public/
folder as it has the default UI that will be shown when your app has no own
public folder. So the names of these 4 folders are the one that we need to to set "files"
to in our package.json
. That way only these folders will be uploaded to the npm registry, all other files will be ignored. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes i get it
please have a look and review |
package.json
Outdated
@@ -11,9 +11,6 @@ | |||
"url": "https://github.com/hoodiehq/hoodie/issues" | |||
}, | |||
"dependencies": { | |||
"@hoodie/admin": "^1.0.1", | |||
"@hoodie/client": "^10.0.0", | |||
"@hoodie/server": "^23.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to put these back in, that’s why the tests are currently failing, see https://travis-ci.org/hoodiehq/hoodie/builds/249421274#L564
@@ -53,6 +50,9 @@ | |||
"engines": { | |||
"node": ">=4" | |||
}, | |||
"files": [ | |||
"bin", "cli", "public", "server" | |||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that looks good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for helping :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, thanks :)
and released :) https://github.com/hoodiehq/hoodie/releases/tag/v28.2.1 |
closes #779