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

Support conditional export resolution with custom resolver #505

Merged
merged 5 commits into from
Feb 2, 2023

Conversation

nick-klaviyo
Copy link
Contributor

Support resolution of conditional export packages with customer resolver. This change implements the solution described in #504

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 2, 2023

Thank you for the PR. Could you check that npm test is succeeding. Seems like eslint wants some changes.
Furthermore, it would be nice to edit the resolve method in index.d.ts to also allow to return the object.

@nick-klaviyo
Copy link
Contributor Author

nick-klaviyo commented Feb 2, 2023

Thank you for the PR. Could you check that npm test is succeeding. Seems like eslint wants some changes. Furthermore, it would be nice to edit the resolve method in index.d.ts to also allow to return the object.

Thanks - I've added the missing comma that was failing eslint and updated index.d.ts - let me know if that additional return type is what you were thinking.

Also: is there additional configuration I need to do locally in order to run eslint? If I run the pretest npm script locally I get thousands of errors across all the project files. Something must be off with my settings 🤔 NVM. I had some additional files eslint was catching

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 2, 2023

let me know if that additional return type is what you were thinking.

I was thinking of { path: string, module?: string } as you can optionally also return a new module name that should be used for lookup.

And maybe add /test/additional-modules/my-es-module/index.js to the .eslintignore to make eslint happy.

@XmiliaH XmiliaH merged commit fe3ab68 into patriksimek:master Feb 2, 2023
@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 2, 2023

Seems good to me now. Thank you.

@nick-klaviyo
Copy link
Contributor Author

Thank you!

@nick-klaviyo nick-klaviyo deleted the fix-conditional-export-resolve branch February 2, 2023 21:39
@nick-klaviyo
Copy link
Contributor Author

@XmiliaH Do you know when this will be released?

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 3, 2023

I am busy over the weekend. Will try to do it on Monday.

@XmiliaH
Copy link
Collaborator

XmiliaH commented Feb 5, 2023

The new version is released

@nick-klaviyo
Copy link
Contributor Author

The new version is released

Thanks!

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