Skip to content

Migrate to unplugin#17

Closed
illright wants to merge 2 commits intolezer-parser:mainfrom
illright:main
Closed

Migrate to unplugin#17
illright wants to merge 2 commits intolezer-parser:mainfrom
illright:main

Conversation

@illright
Copy link

@illright illright commented Nov 8, 2023

I drafted a solution to migrate to unplugin, but ran into some issues in the process. Since we introduce an additional build-time dependency, we need to bundle it to avoid burdening the end user. That means we no longer can simply externalize every dependency and let Node figure it out.

For that reason, I slightly edited the building logic. I decided to replace Rollup with esbuild because it's more out-of-the-box, requires no plugins and less fiddling. That brought me to the problem of the @lezer/lr package which contains a ./dist/constants.js file that actually exports nothing, and isn't mentioned in the exports field of package.json. To proceed with this, we need to remedy those issues in @lezer/lr.

Closes lezer-parser/lezer#48

@illright
Copy link
Author

illright commented Nov 8, 2023

This PR should address the issues I'm facing with the lr package: lezer-parser/lr#64

@marijnh
Copy link
Collaborator

marijnh commented Nov 8, 2023

Since we introduce an additional build-time dependency, we need to bundle it to avoid burdening the end user

Why would you need to bundle a build-time dependency?

I decided to replace Rollup with esbuild

That's not something I'm willing to go along with.

@illright
Copy link
Author

illright commented Nov 8, 2023

Why would you need to bundle a build-time dependency?

Because we don't want to make end users have to install unplugin to use the plugin

That's not something I'm willing to go along with.

Why? And how would you suggest to tackle the bundling of the plugin?

@marijnh
Copy link
Collaborator

marijnh commented Nov 8, 2023

 Because we don't want to make end users have to install unplugin to use the plugin

Isn't that how the npm ecosystem works? You install dependencies when you need them.

@illright
Copy link
Author

illright commented Nov 8, 2023

Isn't that how the npm ecosystem works? You install dependencies when you need them.

Not precisely. You install dependencies when you need them, but it is expected that these dependencies will take of bundling themselves and not require you to do it in your project.

Here's a very popular unplugin, unplugin-icons, installing it doesn't require you to install its build dependency unplugin.

@marijnh
Copy link
Collaborator

marijnh commented Nov 8, 2023

Why would users be required to bundle the unplugin dependency? Can't it just be loaded from node_modules like usual?

@illright
Copy link
Author

illright commented Nov 8, 2023

Not if it's explicitly declared as a dependency, especially for stricter package managers like pnpm.

@marijnh
Copy link
Collaborator

marijnh commented Nov 8, 2023

Obviously it'd have to be declared as a dependency, yes.

@illright
Copy link
Author

illright commented Nov 8, 2023

I sense a lot of resistance both here and in my other PR that you closed. I have no interest in trying to convince you that my approach to libraries is any better, I just want to use Lezer from ESBuild. If you're not willing to accept this PR, I will just create the unplugin in a separate repo and publish it as a separate package.

@illright illright closed this Nov 8, 2023
@marijnh
Copy link
Collaborator

marijnh commented Nov 8, 2023

That's probably for the best.

Coming into a project and reorganizing stuff wholesale to fit your preferred approach is not always going to be welcome.

@illright
Copy link
Author

illright commented Nov 8, 2023

Here it is, in case you want to add a note about it: https://www.npmjs.com/package/unplugin-lezer. It's a shame we couldn't come to a consensus.

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.

Extend bundler support to Webpack and Vite using Unplugin

2 participants

Comments