Skip to content

tsparser: usage of default exported resources #1970

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

fredr
Copy link
Member

@fredr fredr commented Jun 17, 2025

the resolved bind of a default export will have no name, but we still need to add it or it wont be tracked

@fredr fredr requested a review from eandre June 17, 2025 09:00
@fredr fredr self-assigned this Jun 17, 2025
@encore-cla
Copy link

encore-cla bot commented Jun 17, 2025

All committers have signed the CLA.

@fredr fredr changed the title tsparser: usage of default exported sqldb tsparser: usage of default exported resources Jun 17, 2025
@@ -39,7 +39,7 @@ pub const GATEWAY_PARSER: ResourceParser = ResourceParser {
for r in iter_references::<Res>(&module, &names) {
let r = report_and_continue!(r);

let object = match &r.bind_name {
let object = match r.bind_name.ident() {
None => None,
Copy link
Member

Choose a reason for hiding this comment

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

I guess this could resolve the default export, but not sure how difficult that would be to do. If it's tricky maybe add a TODO here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure how object is used, but I've given it a stab, it resolves to something, not sure exactly what we want it to resolve to?

@@ -61,7 +61,7 @@ fn parse_cron_job(
pass: &mut ResourceParseContext,
r: NamedClassResource<DecodedCronJobConfig>,
) -> ParseResult<()> {
let object = match &r.bind_name {
let object = match r.bind_name.ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@@ -46,7 +46,7 @@ pub const OBJECTS_PARSER: ResourceParser = ResourceParser {
let r = report_and_continue!(r);
let cfg = r.config.unwrap_or_default();

let object = match &r.bind_name {
let object = match r.bind_name.ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

});
}
}

{
for r in iter_references::<NamedStaticMethod>(&module, &names) {
let r = report_and_continue!(r);
let object = match &r.bind_name {
let object = match r.bind_name.ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@@ -71,7 +71,7 @@ pub const SUBSCRIPTION_PARSER: ResourceParser = ResourceParser {
spread.err("cannot use ... for PubSub topic reference");
continue;
}
let object = match &r.bind_name {
let object = match r.bind_name.ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@@ -63,7 +63,7 @@ pub const TOPIC_PARSER: ResourceParser = ResourceParser {

for r in iter_references::<PubSubTopicDefinition>(&module, &names) {
let r = report_and_continue!(r);
let object = match &r.bind_name {
let object = match r.bind_name.ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@@ -152,7 +152,7 @@ pub const SQLDB_PARSER: ResourceParser = ResourceParser {
}
};

let object = match &r.bind_name {
let object = match r.bind_name.ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

});
}
}

{
for r in iter_references::<NamedStaticMethod>(&module, &names) {
let r = report_and_continue!(r);
let object = match &r.bind_name {
let object = match r.bind_name.ident() {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Some(id) => pass
let object = match r.bind_name {
BindName::Anonymous => None,
BindName::DefaultExport(ref expr) => {
Copy link
Member

@eandre eandre Jun 18, 2025

Choose a reason for hiding this comment

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

I don't think we should be resolving this by passing in an expression to resolve_obj.

resolve_obj goes into self.expr(...) which goes into a big match statement where we end up at:

            ast::Expr::Ident(expr) => {
                let Some(obj) = self.ident_obj(expr) else {
                    HANDLER.with(|handler| handler.span_err(expr.span, "unknown identifier"));
                    return Type::Basic(Basic::Never);
                };

                let named = Named::new(obj, vec![]);
                Type::Named(named)
            }

This goes into ident_obj(expr):

    fn ident_obj(&self, ident: &ast::Ident) -> Option<Rc<Object>> {
        // Does this represent a type parameter?
        self.state.resolve_module_ident(self.module, ident)
    }
}

And resolve_module_ident checks:

        // Is it a top-level object in this module?
        let ast_id = AstId::from(ident);
        if let Some(obj) = module.data.top_level.get(&ast_id) {
            return Some(obj.clone());
        }

I believe the correct way to resolve default exports is to do the same thing, but check:

        if let Some(obj) = module.data.default_export {
            return Some(obj.clone());
        }

(sorry for the trouble)

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