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

Amend shapeshifter to allow use of dynamic import #171

Merged
merged 1 commit into from Aug 4, 2022

Conversation

debrisapron
Copy link
Contributor

Whilst implementing the standard import syntax (import foo from "./bar") is problematic due to the statically analyzed nature of these imports, the dynamic import syntax works fine in this context. Unfortunately the shift parser does not support dynamic import and actually throws an error when it sees the keyword, so I have implemented a mildly hacky solution which swaps it out before parsing, then swaps it back in again afterwards.

You can see it working by running this code in the REPL:

const { default: confetti } = await import('https://cdn.skypack.dev/canvas-confetti');
confetti();

stack(
  "<C^7 F^7 ~> <Dm7 G7 A7 ~>"
   .every(2, fast(2))
   .voicings(),
  "<c2 f2 g2> <d2 g2 a2 e2>"
).transpose("<0 2 3 4>")

@felixroos
Copy link
Collaborator

felixroos commented Aug 4, 2022

Great!
While this looks a bit hacky, I don't see a problem it might cause. Just did a bit of research and found out that shift only supports ECMAScript up to ES2019, but dynamic import was added in ES2020. So this is currently the only solution with shift. Sadly, shift seems a bit behind atm..

Maybe we could move to another parser / transformer lib in the future, like babel standalone (which is based on acorn, which supports ES2020) or if that's too heavy just the packages we need (@babel/parser @babel/traverse @bable/type @babel/generate + any plugin packages we need, like top level await.

But for now, this PR looks good and is a valuable addition, so thank you :) should we merge it then?

@debrisapron
Copy link
Contributor Author

Let's do it! But yes, I think the permanent solution is to switch to @babel/parser, it's strongly optimized for this kind of quick and simple AST manipulation, extremely battle-tested & well documented + you can even support sexy experimental JS syntax if you like (e.g. https://github.com/tc39/proposal-pipeline-operator)

@felixroos felixroos merged commit b864b1d into tidalcycles:main Aug 4, 2022
@felixroos
Copy link
Collaborator

fixes #168

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.

None yet

2 participants