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

Ignore resolve requests that start with ! #1359

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

mansona
Copy link
Member

@mansona mansona commented Feb 14, 2023

This PR fixes an issue where the @embroider/webpack plugin is trying to resolve things that it probably shouldn't. This problem and the proposed solution was discussed at the webpack weekly meetings and I only have a vague understanding of the cause but I'll do my best to explain in the background section below.

Note: I know that this implementation is likely a bit naive and we can probably fix it a better way.

I will also spend a little bit of time to see if I can create a failing test that this will actually fix but I have a feeling I might need a bit of help considering I don't fully understand the issue 🙈

Edit: this PR now has a failing test that is fixed with the implementation 🎉

Background

To replicate this issue you can run this reproduction repo https://github.com/mainmatter/embroider-peerdependency-bug against the current main of embroider. When you follow the steps in that reproduction you will get the following error:

Module not found: Error: ember-welcome-page is trying to import from !.. but that is not one of its explicit dependencies

dropping a debugger where the error is being thrown you can check the call stack and the issue is related to the fact that the preHandleExternal() is called with a request.specifier of the following:

!../../../../../../../../../opensource/ember/embroider/node_modules/style-loader/dist/runtime/injectStylesIntoStyleTag.js

This issue seems to have been created due to recent refactoring and the consensus seems to be that we just don't want this webpack plugin to deal with anything that starts with a ! and it should fall back to the other resolvers in the stack.

Note: we first said that we should add this to the ignore list in

function isRelevantRequest(request: any): request is Request {
but that function seems to have been removed in recent refactorings 🙃

When I first came across this bug I was able to identify that it was first introduced in this commit: 6f56f0d which is titled improved self-name resolution. I'm not sure exactly what this is trying to do 🤷 but it's the info I was able to glean.

@mansona mansona changed the title ignore resolve requests that start with ! Ignore resolve requests that start with ! Feb 14, 2023
@ef4 ef4 merged commit 565e9e5 into embroider-build:main Feb 15, 2023
This was referenced May 2, 2023
@ef4 ef4 added the bug Something isn't working label May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants