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: try harder to resolve .mjs files for the browser entries #21161

Merged
merged 3 commits into from
Feb 21, 2023

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Feb 20, 2023

Closes #21125

What I did

@tmeasday found that the issue was caused by duplicate bundling of addon preview code.

This was because sometimes the .mjs file was resolved, and sometimes the .js file was resolved.
I've added code to try to find .mjs files for resolved files we know will end up in the browser.

I've tested this in the reproduction, and this fixed the issue.

I suspect this will also have quite a positive effect on the bundle-size, and potentially even on build-time.
Good stuff.

@ndelangen ndelangen self-assigned this Feb 20, 2023
@ndelangen ndelangen added the bug label Feb 20, 2023
@ndelangen ndelangen changed the title after resolve, check FS to see if there's a .mjs version available Fix: try harder to resolve .mjs files for the browser entries Feb 20, 2023
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Will this not be a problem if the addon does not have a .mjs file in dist? We can't assume that all addons will do that, right?

Are you sure that following through more with the technique of stripping the extension (which previously we were doing for the managerEntries but not previewAnnotations) isn't the right solution here?

@ndelangen
Copy link
Member Author

@tmeasday the better solution would be to maybe use enhanced-resolve or something.

I did verify this worked. I was planning on introducing better support for export maps at some point via some method (again enhanced-resolve comes to mind). In fact I experimented with it for a bit, but couldn't make it work within the time-box, and so abandoned it in favor of this solution.

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

OK, makes sense then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: The Primary doc block in an attached MDX file shows invalid code preview
3 participants