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

acorn-walk does not work with injected plugins #746

Open
kristoferbaxter opened this issue Oct 16, 2018 · 2 comments
Open

acorn-walk does not work with injected plugins #746

kristoferbaxter opened this issue Oct 16, 2018 · 2 comments

Comments

@kristoferbaxter
Copy link

Hello,

I've been attempting to use acorn-walk with the acorn-dynamic-import plugin both before and after the version 6 update. So far, have been quite unsuccessful with even the most basic examples.

I created a repo demonstrating the issue: https://github.com/kristoferbaxter/dynamic-walk

Essentially it appears that extensions to base are not leveraged within the simple or ancestor methods.

ampproject/rollup-plugin-closure-compiler#48 was the start of this issue – trying to add support for dynamic import to a rollup plugin for minification with closure compiler.

@marijnh
Copy link
Member

marijnh commented Oct 16, 2018

Good point. I don't think this worked pre-6.0 either. The problem appears to be that the functions exported from acorn-walk default to the base visitor that they close over, which will not be the modified one produced by the inject function exported by acorn-dynamic-import. You can work around it by passing walk.base as a third argument to walk.simple, so that it'll use that one rather than the one it closed over.

I'm not sure how we can work around this in a backwards-compatible way (apart from fragile hacks which depend on this being bound to the module). I think the premise of that inject function is kind of wrong, and it should just take and return a base visitor object, to make it obvious that the user is responsible for passing that to the walker functions.

@kristoferbaxter
Copy link
Author

Thanks for the followup, this issue did exist pre-6.0 as well in my tests. I delayed reporting it since the migration to 6.0 was just beginning when I first encountered it. In retrospect, I should have filed it then.

I think the premise of that inject function is kind of wrong, and it should just take and return a base visitor object, to make it obvious that the user is responsible for passing that to the walker functions.

That seems like a viable path forward to me. Will use the workaround you suggest until an official path forward is created.

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

No branches or pull requests

2 participants