Skip to content

Conversation

@VladimirTechMan
Copy link
Contributor

In the just-released pegjs 0.10.0, they decided to rename method 'buildParser' to 'generate'. Thus, pegjs-loader needs an update to correctly work with this new public version of pegjs.

(The proposed code change keeps backward compatibility with the old method name, just in the case if some people still need to use the previous version of pegjs, for some more time.)

@polgfred
Copy link

polgfred commented Oct 6, 2016

But since you changed the dependency on pegjs to ^0.10.0, the backwards compatibility shouldn't be necessary, right?

@VladimirTechMan
Copy link
Contributor Author

@polgfred Now that most of the people using pegjs should have switched to v. 0.10 anyway (I assume), that part of the code in the patch can be dropped, I have no objections. It was more relevant in the first few days after the initial release of pegjs 0.10, keeping in mind a possibility of some other backward-breaking changes that would keep some people on v. 0.9. And the thing is, if you have a dependency that is not met, but you are using an older version of the package, then npm will warn you about it, but it won't prevent using the dependent package with that older version (with the default conf. settings, at least). Thus, the updated dependency in this case would serve as a natural reminder when using npm, yet people could go with pegjs 0.9 for a little while without having to downgrade the version of pegjs-loader as well.

Right now, it looks like something might have happened on @eploko's side, honestly. I don't know. I hope nothing serious, really. Maybe just some global change in plans and focus. Thus, @polgfred, while I have no problem updating the patch code above, it's just that it won't help us much with the new official npm release of the pegjs-loader, I guess.

Given the long absence of @eploko here. I was considering to start providing a new official npm distro branched from this peg loader. We would just need a new name for it, to avoid clashing with the initial one. If the community is interested in that at all, then I could take care of it, I think.

@TamsynUlthara
Copy link

@VladimirTechMan Yeah, I'm starting to think a fork is necessary.

As far as naming, I was going to suggest peg-loader but that's yet another unmaintained PegJS loader. o_O

How about pegjs-grammar-loader?

We'd also want to get it added to the Webpack documentation's loader list.

@polgfred
Copy link

polgfred commented Oct 7, 2016

I tried to pin my version of pegjs-loader to your fork in package.json, but it didn't actually run npm run build, so the file in lib didn't get built. So having it be in npm proper would be great. I'm not in a hurry to move to 0.10, so I don't need to jump through any hoops at present.

@eploko
Copy link
Owner

eploko commented Oct 7, 2016

Hey all. Thanks a ton for the PR. Sorry for the long delay in merging it, was a bit overwhelmed with other things in my life. I'll merge it all now.

@eploko eploko merged commit 7a65f1c into eploko:master Oct 7, 2016
@TamsynUlthara
Copy link

@eploko Thanks! That's much appreciated. :-)

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.

4 participants