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

Add @glimmer/reference as a virtual package #1513

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

chancancode
Copy link
Contributor

Another fix to #1487

The fix in #1495 turned out to be incomplete and didn't solve my actual problem (which I reduced away in my report). The actual import in the app was for @glimmer/reference, which internally imports from @glimmer/validator. I think because in @glimmer/reference's package.json it has its own dep on @glimmer/validator it escapes this rule and acquired the actual copy in node_modules, causing the duplication.

It seems like this would also be a problem if you, say, imported from @glimmer/runtime (which imports from @glimmer/reference and, in turns, @glimmer/validator)? I don't personally have that particular use case but I could see that happening. Do we just wait and see or is there something more targeted hard-coding that we should do specifically for @glimmer/validator?

@chancancode
Copy link
Contributor Author

Failure seems unrelated

@ef4
Copy link
Contributor

ef4 commented Jul 3, 2023

The more complete thing to do is to go through ember-source and capture the complete list of "packages" that it defines. Possibly per ember-source version. That would be the more reliable thing for us to use here.

For now this addition is good though. Thanks.

@ef4 ef4 merged commit a147aa2 into embroider-build:main Jul 3, 2023
@NullVoxPopuli NullVoxPopuli added the bug Something isn't working label Jul 5, 2023
@chancancode chancancode deleted the patch-1 branch July 13, 2023 17:02
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.

3 participants