Suggestion for update on index exports

[dalefrancis88]

Team, here is a suggestion for how i'd like to update the index entry tests.

There is a little tautology here in the tests but i left it in as I'd like to know everyones thoughts on what we're trying to cover and show. Object.entries will give us the individual prop level comparison with nice outputs and the same check determines that there is EXACTLY what we were expecting to be exported in the expected structure. This could be changed to just be comparing the keys, that way it's more specific and could have a message of exports only the expected keys or something to that affect

[coveralls]

Coverage remained the same at 100.0% when pulling 8722f14baf68f6af099d1d58c5b05b5fec3f2a7d on chore/index-export into f073ee864e8d6f25c1a65b9b86dc93e4b80e32ca on master.

[evilsoft]

I personally prefer the explicit definition and a test for each and as @bennypowers mentioned this will be a little complicated once we are modules and we would probably wanna move them back to the old pattern. But I do see how it can keep the lines down.

Would you feel super bummed if we did not take this approach (at least not now)?

[dalefrancis88]

The intention is not to keep the lines down, the intention is more about being explicit with the testing on the exports.

There are a few ways that this can be implemented but what i'm trying to cover is more the testing strategy and the reasoning. I am happy to keep the requires separate and construct the object. This initial input was more related to the ensuring we were covering the intent. Happy to implement any way that is necessary

[evilsoft]

Ohhh. I gotcha now took a minute to see the value. Lets keep this WIP and think on it for a bit. need to weigh it with this new understanding.

[dalefrancis88]

If you like, i can update it based on the above concerns? it may make it easier to address?

[dalefrancis88]

So @bennypowers the way these imports are being brought in and the object constructed is the same as all the index files already, are you saying this change will need to happen everywhere?

[bennypowers]

@dalefrancis is not a blocker, since the native module story for node is still kind of up in the air. Maybe there will be synchronous dynamic import for node modules, but afaik it's always async.

So yeah my point is sort of tangential, but something to keep in mind across the whole project.

[dalefrancis88]

Cool. Personally i like the object structure of this (hence why i wrote it 😄) But i am going to try to think of a different way that may make @evilsoft more comfortable. I'm trying to avoid just haveing an array of all the keys though and to be explicit with the keys expected, so obv length is out of the question

[dalefrancis88]

@bennypowers @evilsoft I'm not sure where to go from here with this one. I've been thinking about it and I can't think of a nicer version that is able to test all the exports as well as testing all the expected keys and ensuring that there is a failure if something extra is unexpectedly exported. Do you guys have any thoughts on it?

[bennypowers]

Yeah I'm good. It would be better if all require expressions were at the top of the module scope, so that they could be more easily mechanically translated to es modules. As it stands now that will require some manual copypasta.

but as I said above, it's not a critical issue.