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

fix broken entry point in package.json, add development strings #38

Merged
merged 2 commits into from
Apr 26, 2016
Merged

fix broken entry point in package.json, add development strings #38

merged 2 commits into from
Apr 26, 2016

Conversation

amy-langley
Copy link
Contributor

We are having trouble consuming panoptes-client npm package because the main in package.json was set to a file that no longer exists. This pull request fixes that, as well as adding strings to make "development" a meaningful environment name in addition to staging.

The npm package will need to be version-bumped after this change is accepted.

@brian-c
Copy link
Contributor

brian-c commented Apr 26, 2016

@amyrebecca Good catch. Mind deleting the unused root index.js file?

@wgranger This will very likely fix the path resolution issue we couldn't figure out yesterday.

@amy-langley
Copy link
Contributor Author

To be honest this was @camallen's catch and I'm mopping up after, but we were collaborating on fixing the weird behavior of react-native. I'll add the index.js cleanup to my PR, that's also wise.

@amy-langley
Copy link
Contributor Author

amy-langley commented Apr 26, 2016

Wait, is index.js actually unused? it looks like i could say

import {apiClient} from 'panoptes-client' and it should work?

@brian-c
Copy link
Contributor

brian-c commented Apr 26, 2016

It's unused in PFE, and since "main" is defined you can now do `import apiClient from 'panoptes-client'.

Either define "main" or include an index.js, but not both. I'd go the "main" route since index.js as it's currently built includes a bunch of code that you're probably not intending to include.

@amy-langley
Copy link
Contributor Author

That makes sense to me. I've done what you suggested.

@brian-c
Copy link
Contributor

brian-c commented Apr 26, 2016

Awesome, thanks. I'll merge and bump the patch version and publish in just a minute.

@rogerhutchings
Copy link
Contributor

This was a breaking change :(

@brian-c
Copy link
Contributor

brian-c commented Jun 15, 2016

Yes, this fixed a bug that I was unaware people were depending on. Had I known people depended on the broken behavior, I'd've bumped the major. Sorry.

@rogerhutchings
Copy link
Contributor

Ahh, no biggie. I think browserify must fall back to index.js if the entry point is broken, so it appeared to be fine...

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