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

Add missing exports #442

Merged
merged 3 commits into from
Aug 20, 2020
Merged

Add missing exports #442

merged 3 commits into from
Aug 20, 2020

Conversation

dicearr
Copy link
Contributor

@dicearr dicearr commented Aug 20, 2020

While updating Momi I realized that it is not possible to deep import any file which is not exported in the exports object.

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './test/assertions.js' is not defined by "exports"
in /home/diego/dev/fluture/momi/node_modules/fluture/package.json imported
from /home/diego/dev/fluture/momi/test/index.js

I have only added test/assertions and test/arbitraries but I don't know what to do with the types.

I wouldn't export src/* although I am not aware if we have any important project deep importing from it.

@dicearr dicearr requested a review from Avaq August 20, 2020 11:30
@dicearr dicearr self-assigned this Aug 20, 2020
@codecov

This comment has been minimized.

@Avaq
Copy link
Member

Avaq commented Aug 20, 2020

While updating Momi I realized that it is not possible to deep import any file which is not exported in the exports object.

In that case I'd also like to add ./index.js to preserve backwards compatibility.

@Avaq
Copy link
Member

Avaq commented Aug 20, 2020

I don't know what to do with the types.

I don't know what you mean.

@dicearr
Copy link
Contributor Author

dicearr commented Aug 20, 2020

Well, does types need to be imported?

@dicearr
Copy link
Contributor Author

dicearr commented Aug 20, 2020

While updating Momi I realized that it is not possible to deep import any file which is not exported in the exports object.

In that case I'd also like to add ./index.js to preserve backwards compatibility.

I will add it. So, it wouldn't have been a breaking change.

@Avaq
Copy link
Member

Avaq commented Aug 20, 2020

So, it wouldn't have been a breaking change.

It still is for users who were doing import 'fluture'. Just no longer for users who were doing import 'fluture/index.js'.

@Avaq
Copy link
Member

Avaq commented Aug 20, 2020

Well, does types need to be imported?

You mean the TypeScript types? I don't think TypeScript cares about the "exports" at all. It does its own thing. TypeScript has always imported the modular code, even when "main" points somewhere else.

@Avaq Avaq merged commit f1eff52 into master Aug 20, 2020
@Avaq Avaq deleted the diego/add-exports branch August 20, 2020 11:59
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.

2 participants