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

New way to create Brunch plugins #11

Open
urfolomeus opened this issue Jan 25, 2016 · 10 comments
Open

New way to create Brunch plugins #11

urfolomeus opened this issue Jan 25, 2016 · 10 comments

Comments

@urfolomeus
Copy link
Contributor

It looks like there is a new template for creating Brunch plugins.

I discovered this when someone raised an issue on my SeatSaver repo. We were seeing strange behaviour when the elm file was in the watchlist, but everything worked fine when it wasn't. After a bit of poking around (and discussing with the fine folks on the Elixir Slack #phoenix channel) I realised that it was the latest release of Brunch (2.2.0) that had caused the issue and that the method for creating plugins had changed. I hacked out a quick spike to test and it seems to work fine on my environment (albeit much more simple and hardwired to my use case).

I'll hopefully have time to hack out a proper solution tomorrow evening if you like?

@tobyhede
Copy link

semver, how does it even!!?

@polymetis
Copy link

I ran into this on the weekend. Slow to the draw I guess. :P @madscoaducom This is what I was talking about on twitter but I wanted to make a proper bug report. If you need any help feel free to ping me but @urfolomeus 's spike seem to work here as well.

@tobyhede
Copy link

I am a bit rusty on the node ecosystem but happy to help test a PR.

@madsflensted
Copy link
Owner

@urfolomeus Thanks for taking the time to investigate the details.
For now I have added a warning to the README.

I wonder how we should handle the users that stay with brunch < 2.2.0 ...hurray for backwards incompatible changes.

I will try to look at this tonight.

@urfolomeus
Copy link
Contributor Author

@madsflensted I've had a first pass at getting the existing test suite (well a slightly modified one) to pass. You can see it on my spike.

Will give it a go in my Phoenix/Elm app and see how it goes.

Some caveats:

  1. This is very much me just poking about to see how far I can go. Happy to drop that repo entirely and bring the results in here if they're useful.
  2. This is just a first pass, I've not tried it in the Phoenix/Elm app yet to see if it works (will do that now). Also I may have missed some existing functionality.

Hope this helps :)

@urfolomeus
Copy link
Contributor Author

Um well, it would seem I have more work to do 😄

(not quite working yet)

@madsflensted
Copy link
Owner

I have been doing some debugging in brunch and the plugin.
I have found that brunch supports the old plugin model still, but it seems that somewhere (I have not found it yet) - the fact that elm-brunch returns data = null to the callback, directs the brunch code to try and access '.path' of undefined.

If I make a slight change to this line and call the callback with "" instead of null on success - the error goes away.

return callback(error, error ? stderr : "");

It also looks like all the files are generated properly, but I do not feel comfortable about this.

@polymetis @urfolomeus and @tobyhede could you try this hack in your local node_modules/elm-brunch/index.js and see if that fixes it for your? (when you run with brunch 2.2.1)

@urfolomeus
Copy link
Contributor Author

Works for me 😊 Nice find and much simpler than my approach.

@madsflensted
Copy link
Owner

Ok - I have published a 0.4.2 with this workaround. I suspect that the side effect is that the rest of the pipeline is run with a empty source string, but this is probably a minor issue for Elm projects.

Going forward I think re-writing to the new plugin format, is the way to go.

@tobyhede
Copy link

Works for me too

On 27 January 2016 at 18:04, Mads Flensted-Urech [email protected]
wrote:

Ok - I have published a 0.4.2 with this workaround. I suspect that the
side effect is that the rest of the pipeline is run with a empty source
string, but this is probably a minor issue for Elm projects.

Going forward I think re-writing to the new plugin format, is the way to
go.


Reply to this email directly or view it on GitHub
#11 (comment)
.

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

4 participants