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

fix: support JSON entrypoints in JSR packages #476

Merged
merged 2 commits into from
May 16, 2024

Conversation

dsherret
Copy link
Member

No description provided.

@dsherret dsherret requested a review from bartlomieju May 16, 2024 00:42
@@ -3019,8 +3022,6 @@ struct PendingState<'a> {
pending: FuturesOrdered<PendingInfoFuture<'a>>,
jsr: PendingJsrState,
npm: PendingNpmState,
pending_specifiers:
HashMap<ModuleSpecifier, HashSet<Option<AttributeTypeWithRange>>>,
Copy link
Member Author

@dsherret dsherret May 16, 2024

Choose a reason for hiding this comment

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

What it was doing here was quite broken handling of attribute types. The change I made is also broken, but slightly better. To fix this properly we need to key on attribute type in addition to specifier for pending loads, but that's a big change for the future (that said, what we probably want to do is not allow two imports to the same specifier with different attribute types for the time being, but I think that's maybe handled by v8?).

Copy link
Member

Choose a reason for hiding this comment

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

It's not really handled by V8 - given (specifier, attributes) it is embedders responsibility to provide the proper source for the module.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, ok, I'll do a follow-up to fix. Before it was actually only sometimes erroring.

Copy link
Contributor

Choose a reason for hiding this comment

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

Back in the wmr days we serialized the import attributes to query params and added that to the file URL. That worked quite well and allowed us to keep the cache logic flat

@dsherret dsherret merged commit 70085ad into denoland:main May 16, 2024
16 checks passed
@dsherret dsherret deleted the attribute_type_loader branch May 16, 2024 00:58
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.

None yet

3 participants