-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Make import stripping smarter to resolve #5019 #5114
Conversation
LGTM, lets add a quick node-test smoke testing this transform, then we can ship-it. |
70f5079
to
af0aacc
Compare
@stefanpenner @runspired can this be merge to resolve #5016 also in 2.15.0 |
@xomaczar unfortunately, not until it has tests. cc @runspired |
I don't mind pushing it forward, if u can clarify what kind of tests you are looking for. I assume something similar to the current e-d generators |
@xomaczar a unit test, which uses babel directly, and configures it to use the above plugin, on various inputs confirming functionality works as expected. |
af0aacc
to
1db72d7
Compare
@stefanpenner thanks for the info @runspired thx for impl |
@runspired thanks for the test! It does look like the new test is failing on CI, mind taking a look?
|
@stefanpenner figured I'd push my working state in case others wanted to contribute. I figured out why the test fails though, babel output doesn't create the same AST for string transpile as it does for file transpile. Talked with @rwjblue and he pointed me to broccoli-test-helpers. Getting that working now. |
3857d55
to
a347c61
Compare
a347c61
to
10b1828
Compare
The appveyor log suggests it was actually successful ... |
let specifier = imports[i].imported; | ||
|
||
if (!specifier) { | ||
debugger; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i suspect this doesn't want to be here
10b1828
to
4799bf5
Compare
4799bf5
to
f3bfad8
Compare
Merge? |
@locks are you able to backport and release as well? |
@stefanpenner I'll only be able to do it on Sunday, if there's urgency take it away 💪 otherwise I'll tackle it. |
When is the expected release? |
Sorry, haven't had free weekends to handle this. I put up a backport PR at #5188. |
Previously we would only strip import declarations when all imports from a given module were previously filtered out by another plugin. This PR allows us to strip filtered imports from declarations without stripping the entire declaration if other imports are still needed.