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

prioritize import specigier over packagejson exported type #68

Merged

Conversation

iambumblehead
Copy link
Owner

@@ -318,7 +318,9 @@ const esmparse = (spec, specifier, opts = {}) => {
// if dynamic 'spectype', lookup 'commonjs' or 'module'
// according to package.json
specname = specname === spectype
? getspectypenamedexportdefault(opts.packagejsontype)
? isDirPathRe.test(specifier)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole logic looks a bit... suspicious for me - it's a kind of mix of 'how to resolve exact import' (exports field logic) and 'how to detect module type - CJS or ESM' (type field). but, if it works - let's merge

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree :/

@iambumblehead
Copy link
Owner Author

"specifier" here is basically a path such as opanai/path.js "path.js", a subpath such as opanai#subpath or a keyword-specifier such as "import" or "node"

https://github.com/iambumblehead/resolvewithplus/blob/main/resolvewithplus.js#L517

pspecifier ? './' + pspecifier : specimport

@iambumblehead
Copy link
Owner Author

esm is ... very difficult

@iambumblehead iambumblehead merged commit ccac7b7 into main Jan 24, 2025
5 checks passed
@iambumblehead iambumblehead deleted the prioritize-import-specifier-over-packagejson-exported-type branch January 24, 2025 20:54
@koshic
Copy link
Collaborator

koshic commented Jan 24, 2025

Thx for explanation - now that code looks clear. BTW, ESM and resolution logic... they are simple, BUT if you can image the whole possible scenarios at once :)

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.

3 participants