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

feat: propagate external modules to swc Bundler #95

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

manzt
Copy link
Contributor

@manzt manzt commented Mar 19, 2023

Towards #51

Collects any Source::External module objects from the ModuleGraph and propagrates them as external modules to the swc bundler. Previously, a graph with external modules would throw a runtime exception.

This shouldn't introduce any changes in behavior to the TypeScript API, since the loader since the JsLoader from createCache doesn't expose a way to mark external dependencies.

@CLAassistant
Copy link

CLAassistant commented Mar 19, 2023

CLA assistant check
All committers have signed the CLA.

@manzt
Copy link
Contributor Author

manzt commented Mar 20, 2023

In a follow up, trying to think of how to support Source::External that are resolved to via a custom graph resolve (e.g., something based on import-maps). Right now, I think for the following:

import * as foo from "foo";

export const bar = () => foo();
{
  "imports": { "foo": "https://example.com/foo.js" }
}

if "https://example.com/foo.js" were to be marked as Source::External in the graph, the Load impl for BundlerLoader would need to rewrite the "foo" import to the full resolved "https://example.com/foo.js" in order to stop swc from trying to resolve foo.

@bartlomieju bartlomieju requested a review from dsherret March 20, 2023 15:32
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@dsherret dsherret merged commit 9b1f9cf into denoland:main Jul 25, 2023
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