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

Allow to disable validation of "Default condition should be last one" #160

Closed
hyf0 opened this issue May 15, 2024 · 3 comments · Fixed by #171
Closed

Allow to disable validation of "Default condition should be last one" #160

hyf0 opened this issue May 15, 2024 · 3 comments · Fixed by #171
Assignees

Comments

@hyf0
Copy link
Contributor

hyf0 commented May 15, 2024

Related code:

oxc-resolver/src/lib.rs

Lines 1464 to 1468 in e04ca0f

let is_default = matches!(key, ImportExportKey::CustomCondition(condition) if condition == "default");
if i < target.len() - 1 && is_default {
return Err(ResolveError::InvalidPackageConfigDefault(
package_url.join("package.json"),
));

The current behavior of oxc-resolver is correct based on the spec, but not flexable.

"default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last. link

Though spec said the "default" should always come last, but nodejs actually doesn't do strong validation on it. If "default` shows , it will be matched once detecting it.

Context:

Related:

@hyf0
Copy link
Contributor Author

hyf0 commented May 15, 2024

I personally intend to follow the spec. But this case is tricky, nodejs itself doesn't follow the spec due to reasons I don't know. For maximum compatibility, rolldown needs to follow the esbuild's behavior.

@zhusjfaker

This comment was marked as off-topic.

@Boshen
Copy link
Member

Boshen commented May 15, 2024

The spec never mentioned the logic where "default" must be last or it should throw an error.

image

Hence enhanced-resolve is the one not conforming to the spec.


statement for being the last: https://nodejs.org/docs/v20.13.1/api/packages.html#conditional-exports

image

should, not must.

@Boshen Boshen self-assigned this May 15, 2024
Boshen added a commit that referenced this issue May 27, 2024
…e last one

closes #160

The spec never mentioned the logic where "default" must be last or it should throw an error.

https://nodejs.org/api/esm.html#resolution-and-loading-algorithm

`enhanced-resolve` took the meaning from https://nodejs.org/docs/v20.13.1/api/packages.html#conditional-exports

"This condition should always come last."

This statement is not part of the specification, it is a recommendation.
Boshen added a commit that referenced this issue May 27, 2024
…e last one

closes #160

The spec never mentioned the logic where "default" must be last or it should throw an error.

https://nodejs.org/api/esm.html#resolution-and-loading-algorithm

`enhanced-resolve` took the meaning from https://nodejs.org/docs/v20.13.1/api/packages.html#conditional-exports

"This condition should always come last."

This statement is not part of the specification, it is a recommendation.
Boshen added a commit that referenced this issue May 27, 2024
…e last one (#171)

closes #160

The spec never mentioned the logic where "default" must be last or it should throw an error.

https://nodejs.org/api/esm.html#resolution-and-loading-algorithm

`enhanced-resolve` took the meaning from https://nodejs.org/docs/v20.13.1/api/packages.html#conditional-exports

"This condition should always come last."

This statement is not part of the specification, it is a recommendation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants