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

Chore: simplify plugin import syntax #3090

Merged
merged 2 commits into from
Aug 9, 2023

Conversation

ffxsam
Copy link
Contributor

@ffxsam ffxsam commented Aug 8, 2023

Short description

TypeScript was complaining about not being able to find the declarations, so I fixed the exports map and tested it. Now I get full TS + autocompletion for plugin constructors.

This might've been an issue with VS Code misbehaving, so this PR is now just a bit of polish rather than a fix. I added a shorter way to import plugins, e.g.: import SomePlugin from 'wavesurfer.js/plugins/some-plugin'

Checklist

  • This PR is covered by e2e tests
  • It introduces no breaking API changes

@katspaugh
Copy link
Owner

Hey Sam, thanks for the PR!

Unfortunately, this change will most certainly break people's code.

In the readme and on the website, there are examples of how to correctly import plugins, namely:

import Regions from 'wavesurfer.js/dist/plugins/regions.js'

As you can see, it should have a .js extension. With your changes, this type of import will likely result in a double extension (e.g. regions.js.esm.js), which won't work.

If you prefer to import regions w/o an extension and with a shorter path, which I fully agree is nicer, we should add an additional mapping to the package.json, not replace an existing, working, mapping.

@ffxsam
Copy link
Contributor Author

ffxsam commented Aug 9, 2023

Ah, right! Maybe this is something we can phase out in a future major version, then. I just pushed an update.

I feel like this PR started from what I thought was a bug. I swear the type declarations were not working when I tried the dist/plugins/*.js format, but now I'm thinking VS Code was experiencing a hiccup.

I'll edit the PR to reflect that this is not a fix.

@ffxsam ffxsam changed the title Fix: exports map for plugins Chore: simplify plugin import syntax Aug 9, 2023
@katspaugh
Copy link
Owner

Thanks Sam, this looks good!

@katspaugh katspaugh merged commit 2313458 into katspaugh:main Aug 9, 2023
@ffxsam ffxsam deleted the fix/exports-map branch August 9, 2023 16:03
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