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 snippet resolving #600

Merged
merged 1 commit into from
Nov 5, 2020
Merged

fix snippet resolving #600

merged 1 commit into from
Nov 5, 2020

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Nov 5, 2020

#593 broke some cases when you're parsing component snippts in packageRules at an early point in the build when the app itself doesn't have a package.json yet. This fixes the problem by ensuring we don't try to discover the owningPackage for our own stuff when we know it's owned by the app.

#593 broke some cases when you're parsing component snippts in packageRules at an early point in the build when the app itself doesn't have a package.json yet. This fixes the problem by ensuring we don't try to discover the owningPackage for our own stuff when we know it's owned by the app.
@ef4 ef4 merged commit c175cc0 into master Nov 5, 2020
@ef4 ef4 deleted the snippet-resolve branch November 5, 2020 08:42
@simonihmig
Copy link
Collaborator

This seems to also fix a real issue in an app, not only related to snippets in package rules. Wehen updating an app to 0.30.0 from 0.28.0, the build started to fail with The "path" argument must be of type string. Received undefined, due to packageRuntimeName being undefined. pkg was an object, with dependencies, fastboot and embroider-fastboot being the only keys there, but no name. Patching my node_modules with the changes here fixed the issue.

absPath was referring to https://github.com/tildeio/ember-element-helper/blob/main/app/components/-dynamic-element.js, don't know if there was something specific to that or not, as I didn't investigate this further. Just fyi!

@ef4
Copy link
Contributor Author

ef4 commented Nov 5, 2020

Yup, that's the same bug. We have a built-in rule that has a snippet for that component:

dependsOnComponents: ['{{-dynamic-element}}', '{{-dynamic-element-alt}}'],

@simonihmig
Copy link
Collaborator

We have a built-in rule that has a snippet for that component:

Yup, I added that! :)

However I was not sure how this plays together. But thanks for confirming!

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