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: allow any tsconfig/jsconfig path aliases #320

Closed
wants to merge 14 commits into from

Conversation

mcansh
Copy link
Collaborator

@mcansh mcansh commented Oct 13, 2021

most of this is built into esbuild itself, except how we're handling external modules and we need to manually update the paths in the mdx plugin

Signed-off-by: Logan McAnsh [email protected]

@mcansh mcansh changed the title feat: allow any tsconfig/jsconfig path alias feat: allow any tsconfig/jsconfig path aliases Oct 13, 2021
@mcansh mcansh requested review from mjackson and jacob-ebey and removed request for mjackson and jacob-ebey October 14, 2021 16:40
Signed-off-by: Logan McAnsh <[email protected]>
@mcansh mcansh requested review from jacob-ebey and removed request for mjackson October 29, 2021 14:38
@@ -29,6 +28,7 @@ export default function App() {
<p>This page was rendered at {data.date.toLocaleString()}</p>
</footer>
<Scripts />
<LiveReload />
Copy link
Member

Choose a reason for hiding this comment

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

Can you please make this change in a separate PR if it still needs to be done?

@@ -351,8 +351,20 @@ async function createServerBuild(
});
}

function isBareModuleId(id: string): boolean {
return !id.startsWith(".") && !id.startsWith("~") && !path.isAbsolute(id);
function isBareModuleId(
Copy link
Member

Choose a reason for hiding this comment

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

Instead of mixing bare module id detection code with path prefix detection, let's make a separate function called isPrefixedModuleId or something like that and include the ~ check there as well.

@@ -0,0 +1,4 @@
/.cache
Copy link
Member

Choose a reason for hiding this comment

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

Can we add the tutorial-js fixture in another PR? Why does it need to be part of this work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for sure, just added it to make sure it all works 👍

@mjackson
Copy link
Member

mjackson commented Nov 9, 2021

Don't feel like you have to fix all this right now @mcansh. I'd say let's table this work for now and move onto higher priority stuff for the 1.0 launch. I just wanted to leave some notes for when we come back to it in the future.

@mjackson mjackson closed this Nov 9, 2021
@mcansh mcansh deleted the logan/rem-400-tsconfig-paths branch December 17, 2021 18:38
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