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

Rely on index.js instead of index.json #6

Merged
merged 1 commit into from
Dec 24, 2015

Conversation

rgbkrk
Copy link
Contributor

@rgbkrk rgbkrk commented Dec 24, 2015

As noted in Qix-/color#69, this package currently requires webpack and react-native users to have to do custom things to require this package in their applications, due to the top level module coming straight from .json. This switches over to a simple index.js file.

I ❤️ color, so I'd love to keep using it now that I'm using webpack.

dy added a commit that referenced this pull request Dec 24, 2015
It is the issue of webpack, not the color-name, but that is fourth complaint. Enough.
@dy dy merged commit b482f29 into colorjs:master Dec 24, 2015
@rgbkrk rgbkrk deleted the index.js-round-2 branch December 24, 2015 06:11
@rgbkrk
Copy link
Contributor Author

rgbkrk commented Dec 24, 2015

Thanks for shipping this!

@MoOx
Copy link

MoOx commented Dec 24, 2015

Not sure that deliverying this as a patch was a good idea (eg: someone can hardcode the reference to the json for some "you don't want to know" reason).
Looks like a breaking change.

@rgbkrk
Copy link
Contributor Author

rgbkrk commented Dec 24, 2015

Looks like a breaking change.

That's a fair assessment. If the exported interface were changed, I would say this would be a major version change. Since this changes the implementation and not the exported interface on a require, this would adhere to a minor version bump, which is what @dfcreative did - 1.1.0 had this change. 1.1.1 was a tiny fix to that. I should have mentioned that in the other issue, I provided 1.1.1 to let others know what to pin.

screenshot 2015-12-24 10 03 52

Instead of me squabbling about whether or not someone did a "you don't want to know" hardcoded dependency on a file structure without pinning the version, I dug around to see uses of the package:

All the libraries that rely on color-name do a direct require:

There are only two other repos with direct reference to color-name it's part of an npm shrinkwrap:

@rgbkrk
Copy link
Contributor Author

rgbkrk commented Dec 24, 2015

All this being said, after digging through previous issues, if we want index.json available in the published package, this can go in the package.json by writing out the json directly as a prepublish step:

diff --git a/package.json b/package.json
index 52ec3b1..f1a7001 100644
--- a/package.json
+++ b/package.json
@@ -4,10 +4,12 @@
   "description": "A list of color names and it’s values",
   "main": "index.js",
   "scripts": {
+    "prepublish": "node -e 'process.stdout.write(JSON.stringify(require(\".\")))' > 'index.json'",
     "test": "node test.js"
   },
   "files": [
-    "index.js"
+    "index.js",
+    "index.json"
   ],
   "repository": {
     "type": "git",

No fancy dependencies there and it runs before a publish, based on how npm works as a build tool.

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.

3 participants