feat: Add support for multiple configurations in package.json

[rogerfitz]

• Please check if the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

• What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  feature

• What is the current behavior? (You can also link to an open issue here)
  Currently package.json does not support env's and inherits properties following parent vs nested logic introduced in feat: improve configuration from package.json #606

• What is the new behavior (if this is a feature change)?
  Adds support for package.json idyll to have multiple options. Selectable by API and command line env with functionality as described in Add support for multiple configurations in package.json #640

• Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
  No

• Other information:

• Did I make the test case too big with the node_modules being included? I can work on trimming that if needed
• Also any thoughts on how best to test the different cases in the test? I did a test where outer-project and inner-project have same environments defined but I wasn't sure how best to have it test the other permutations like we talked about in Add support for multiple configurations in package.json #640. Either parent has env but nested doesn't and throws error or parent doesn't have env but local does so no error.
• I modified searchParentDirectories to return the file rather than the file.idyll so I could use things more uniformly between parent file vs local

[mathisonian]

Thanks so much for this @rogerfitz! I'm reviewing now and will leave comments as I go. I believe you should be able to safely exclude the node_modules folder from the tests (assuming that all of the dependencies are already available in the project somewhere, if not we can talk through the best way to handle that).

[mathisonian]

If you end up changing the package.json based on my comment above, re-run yarn to regenerate this file.

[rogerfitz]

I think I did this right but not 100% sure. Quite a large diff but that could be expected from full regenerate? I copied yarn.lock from idyll master and then ran yarn install

[mathisonian]

Big diff for this file - is this a full re-write or just Git being annoying? If you can point to the parts that changed it will be helpful

[mathisonian]

Is the change to searchParentDirectories the only existing functionality that changed?

[rogerfitz]

Yes that and then the new functionality. Will clean up based on your other comment about that one liner

[rogerfitz]

Just pushed a merged version of this. I think Git was just being funny, I handled the merge by copying idyll master's index.js over and making the edits by copying lines from my fork into that so I didn't miss anything. Should be relatively straightforward to see the line changes but the diff is ugly

[mathisonian]

There was a small change to idyll-cli/src/index.js recently that I think is giving the merge conflict. I believe its just this one liner: 48b91bf#diff-7992088eddfefcd56354965a72d75010b7c7aad4dabaff28a54b378a479ed3ff

[rogerfitz]

All those comments should be resolved. Let me know if there's anything else that needs changing.

Anything else you think would be a good issue for me to look at? A while ago I thought I saw something about adding a leaflet component that looked interesting (I'm starting to work with leaflet on some other projects right now) but I could be mistaken. Happy to look deeper into that husky error or windows development issues I had too if that sounds good to you

[mathisonian]

Nice, thanks @rogerfitz! I'll review the new changes and get this merged shortly.

Some type of [Map /] component has been at the top of the list, so a leaflet component would be very welcome. There's also a very basic example using Mapbox.

Another issue that would be helpful is #694. This one should be a relatively quick fix - it seems like there's an issues with the latest node, I believe related to the specific version of react-tooltip that the runtime depends on. The solution likely is either to upgrade that dependency or drop it.

For the husky errors fixes are also welcome. It would also be good to just open an issue on them for visibility if others are running into the same problem

[rogerfitz]

Thanks sounds good. I'll take a stab at #694 first and open a husky issue once I replicate it to see if others have the issues too before looking into [Map /] more

[mathisonian]

Awesome, thanks @rogerfitz! I have a temporary fix for #649 that I'm going to PR and merge soon because I need it for some user studies that are running tomorrow, but if you figure out a way to solve the problem without nuking the dependency completely that would still be appreciated

[mathisonian]

Just merged, thanks @rogerfitz! Since you've made a substantial contribution I've invited you to join as a member of the Idyll organization on GitHub, per our policy.

In the future you can work off of branches directly on this repo rather than forks, which will make it easier for us to collaborate if we both want to push changes to the same PR.

[rogerfitz]

Oh cool thanks!