i started down this path as part of the esm conversion, butbacked away from it because of json based configs. i'm not sure that i fully explored that need at the time and, without some further consideration, i'm not even sure i proved that it was a valid use case to account for.

i would want to at least dig deep enough to prove whether that is a valid case since i dont recall the full context right now. if it is, that is the one additional use case i'd want to make sure is considered.

the fact that it wasnt handled at the time of the esm conversion was simply a matter of deferring the change in order to get the rest of the conversion out the door. we always intended for this to be a follow up, so appreciate the help to get this moving,@dominykas (and@bryanjtc)!

From the linked comment:

import assertions should be available, but would require knowing the format of the config file before loading it

Yeah, that's what I did - I detect JSON based on theextname and add an import assertion. I don't think having a file without an extension used to work withrequire(), so extension based detection should be OK? And in that case - the only question that I'd have is whether we trust the import assertions - an alternative is to fall back torequire oror JSON.parse for JSON?

Did I miss anything? This is all that I saw covered by tests... Happy to add whatever is missing.

i think this exercises enough for me to feel good about this change since the imports are actually happening within the scope of these unit tests. we have a couple of cases in our integration test, but i dont think additional tests there would add more regression protection in this case. i cant think of anything outside of what you've handled, so this looks great to me

🎉 This PR is included in version 22.0.7 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Yoursemantic-release bot 📦🚀

Thanks!

@dominykas couldthis error be related to this change? i forgot to consider the windows case that requires a proper url and we dont have the windows pipeline working for this repo yet to help catch issues like that. since you are most familiar with your change, i'm curious what insight you could provide for this one. thanks!

fromsemantic-release/commit-analyzer#537 (comment):

Since I added JSON module support yesterday, I just triedfixing the issue in semantic-release by using that library, which works well overall, except there's a test that tries to import a node_modules package containing a single index.json file (and no package.json). Currently, the library isn't able to handle that (andit is not specified in the NodeJS docs, although require seems to handle it correctly), but I think it wouldn't even work with an export field pointing to that file in package.json.

Unfortunately, I won't be available in the next few days so I won't be able to work on that issue. Maybe it would be worth prepending file:// in the meantime until I can tackle that issue in import-from-esm?

i'm ok with either approach. if import-from-esm will work well in this situation, i see benefit in the consistency. if that delays things, the alternative approach makes sense to me. we can always follow up to make them consistent as a follow up once getting the situation fixed.

there's a test that tries to import a node_modules package containing a single index.json file (and no package.json). Currently, the library isn't able to handle that (andit is not specified in the NodeJS docs, although require seems to handle it correctly), but I think it wouldn't even work with an export field pointing to that file in package.json.

@sheerlox i could use help understanding this.the only test that i find mentioning anindex.json does also provide apackage.json. and to clarify, is the complexity the name of the file, the lack of a package.json, or a lack of node_modules?

fromsemantic-release/commit-analyzer#537 (comment)

sorry - I do not have Windows to test this myself either.

understood and not a problem. it really comes down to the lack of a windows pipeline for the project, which is on us. we'dstarted an effort to get that back in place, but i failed to see it through fully. this can be motivation to revisit.

I'm on Windows. What needs doing? :)

@seebeen the remaining windows compatibility details would be related to this PR. if you want to test out the current state to confirm the problem and maybe try out the proposed options for fixing, that could be really helpful. we should also work toward getting the windows workflow that i mentioned above into the mix so that we can avoid this sort of situation going forward.

@sheerlox i could use help understanding this.the only test that i find mentioning anindex.json does also provide apackage.json. and to clarify, is the complexity the name of the file, the lack of a package.json, or a lack of node_modules?

Sorry for the delay. Indeed the test I was referring to has a package.json: the complexity is that thepackage.json does not provide amain field, and also thatimport-from-esm relies on extension detection to fallback torequire when loading JSON files. Since the test tries to load an NPM package, this detection cannot occur and the import fails. I haven't checked if providing amain field would change anything, but I'll get to this first thing tomorrow and follow up here. I have a Windows dual-boot for this kind of issue, so I'll be able to make sure it works as expected.

Forgot to mention that I fixed the underlying issue inimport-from-esm today and prepared a PR that should fix the issue at hand withsemantic-release:#3062

@mcmiddle592 the issue you reported insemantic-release/commit-analyzer#537 (comment) should be resolved byhttps://github.com/semantic-release/semantic-release/releases/tag/v22.0.8 since it was most likely introduced by an oversight in this change. would really appreciate it if you could confirm if this new version resolves the issue for you. thanks again for reporting!

@travi@sheerlox it is working now. Thanks for all the help!