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

Fix #3156 #3272

Closed
wants to merge 1 commit into from
Closed

Fix #3156 #3272

wants to merge 1 commit into from

Conversation

samuelgozi
Copy link

@samuelgozi samuelgozi commented Jul 26, 2019

.posthtmlrc plugins can now have true instead of an empty object when no configuration is needed.
This conforms to the documentation: https://parceljs.org/html.html#posthtml.

↪️ Pull Request

This resolves #3156 (That is already closed, but not fixed).

So when using posthtml plugins, sometimes no configuration is required, and according to parcel's official documentation

If there are no options for a plugin, just set it to true instead.

The problem was that for some reason(I don't know why) the object for the settings of each plugin is modified by using Object.assign(pluginsSettings, settingsToAdd).
However, when Object.assign is passed a boolean as the first argument it fails silently(technically the operation is performed, but its still a boolean, so there is no access to those props anymore).

You can see the code for the fix, but basically, I did it that way in order to avoid an if check.

🚨 Test instructions

To be honest im not familiar enough with the internals of Parcel to do a good test.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

`.posthtmlrc` plugins can now have `true` instead of an empty object when no configuration is needed.
This conforms to the documentation: https://parceljs.org/html.html#posthtml.
@samuelgozi
Copy link
Author

For some reason, I'm just not able to run tests on my machine without it failing(on master), so if someone could give me a hand making the tests work on my machine I'll be grateful.

@DeMoorJasper
Copy link
Member

gonna lose this as parcel 1 is no longer receiving updates.

Feel free to check and implement this for v2

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