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.

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

Use files field in package.json instead of .npmignore #415

Merged
merged 2 commits into from
Feb 10, 2017

Conversation

sapegin
Copy link
Member

@sapegin sapegin commented Feb 3, 2017

That’s what you publish to npm now:

image 2017-02-03 at 7 32 54 pm

Most of this files aren‘t necessary. I’m not sure about flow files though.

@kof
Copy link
Member

kof commented Feb 3, 2017

I am not sure what to do with docs.
flow-typed needs to be in the package right now

@kof
Copy link
Member

kof commented Feb 3, 2017

also changelog is nice to have in the package

@kof
Copy link
Member

kof commented Feb 3, 2017

and license

@kof
Copy link
Member

kof commented Feb 3, 2017

and readme)))

@sapegin
Copy link
Member Author

sapegin commented Feb 3, 2017

flow-typed or .flowconfig or both?

I never publish docs but some people do. So it‘s up to you ;-)

Readme, changelog and License should be added automatically: https://docs.npmjs.com/files/package.json#files

@kof
Copy link
Member

kof commented Feb 3, 2017

Are there any reasoning about what to include and what not to? I found it useful to be able to read quickly the docs right from node_modules. But maybe there is a general consensus to only include code?

@sapegin
Copy link
Member Author

sapegin commented Feb 3, 2017

I don’t know about that. But thing like tests, linter configs, etc. shouldn’t bloat node_modules ;-)

@sapegin
Copy link
Member Author

sapegin commented Feb 3, 2017

I’ve heard from someone that they read docs in node_modules while they are offline ;-)

@kof
Copy link
Member

kof commented Feb 3, 2017

Maybe, sometimes tests are saying more than docs …

@kof
Copy link
Member

kof commented Feb 3, 2017

I totally agree regarding everything dev related, not sure about tests though…

@kof
Copy link
Member

kof commented Feb 4, 2017

Ok so lets clarify what needs to be included for webpack 2, otherwise readme.md, license, changelog, flow-typed, dist, lib should be included I think.

@sapegin
Copy link
Member Author

sapegin commented Feb 4, 2017

As I understand you need to use module field in package.json, it should point to ES6 version of your code. You can’t use anything ES6+ here (you’ll have to transpile) and imports should be there.

@sapegin
Copy link
Member Author

sapegin commented Feb 4, 2017

@sapegin
Copy link
Member Author

sapegin commented Feb 6, 2017

I’ve added flow-typed. Readme.md, license, changelog are defaults. Webpack 2 probably needs some experimentation.

@sapegin
Copy link
Member Author

sapegin commented Feb 10, 2017

@kof
Copy link
Member

kof commented Feb 10, 2017

Yeah, sounds right. So we are actually done here, right?

@kof
Copy link
Member

kof commented Feb 10, 2017

Ah no, module field…

@sapegin
Copy link
Member Author

sapegin commented Feb 10, 2017

I think module field would be out of scope of this PR, because it’d require extra Babel configuration.

@kof
Copy link
Member

kof commented Feb 10, 2017

ok

@kof kof merged commit 124e001 into cssinjs:master Feb 10, 2017
@kof
Copy link
Member

kof commented Feb 10, 2017

merged, thanks!

@sapegin sapegin deleted the files branch February 10, 2017 10:28
@sapegin
Copy link
Member Author

sapegin commented Feb 10, 2017

Cool, could you do the same for other JSS projects? Or we can setup mrm for that ;-)

kof added a commit that referenced this pull request Feb 10, 2017
@kof
Copy link
Member

kof commented Feb 10, 2017

Yes, exactly my plan, need to try mrm for that and lots of other things. Though maybe I will need a separate repository for that common stuff for "plugins", because they are a bit different than the core.

@sapegin
Copy link
Member Author

sapegin commented Feb 10, 2017

Most probably. Have you seen webpack-defaults?

@kof
Copy link
Member

kof commented Feb 10, 2017

nope, sounds like a good place to learn from, most probably would need to change a lot, but could use it as a starting point, right?

@sapegin
Copy link
Member Author

sapegin commented Feb 10, 2017

Yeah, sure. It’s not integrated into any project yet. Let me know if you need any help ;-)

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