Skip to content

Make compatible with latest rollup. Fixes #1. #2

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

Merged
merged 2 commits into from
Oct 31, 2018

Conversation

iamDecode
Copy link
Contributor

This plugin did not work with the latest rollup due changes in the signature the signature for ongenerate(see here). Rather than an array, an object is used to store modules. Hence issue #1 occurs.

Object.keys would not work, as we cannot guarantee the proper order of keys. However, in case the keys of the object are strings, Object.getOwnPropertyNames should yield keys in property creation order (see link). Using this method we can restore functionality of this plugin for recent versions of rollup.

Let me know what you think. I haven't tested this thoroughly, but for my project it works fine.

@danburzo
Copy link
Collaborator

Thanks for this! I will also try it in some projects we're using it. I did not stay up to date with the latest Rollup news, and this seems like a good compromise for now. Can we check if module is an Array (with Array.isArray) or an Object and, depending on the type, get the list of modules appropriately? This will help us support a wider range of Rollup versions.

I guess ultimately we'll need to use the generateBundle hook instead of ongenerate / onwrite.

@iamDecode iamDecode force-pushed the decode/rollup-compatibility branch from eb8caeb to a93b61b Compare October 30, 2018 16:42
@iamDecode
Copy link
Contributor Author

Sure thing, PR is updated :)

@danburzo danburzo merged commit e96a462 into Evercoder:master Oct 31, 2018
@danburzo
Copy link
Collaborator

Excellent, thank you!

@danburzo
Copy link
Collaborator

Tested on Evercoder/uiuiui, the order in the CSS is preserved with the upgrade to the latest Rollup version. 👍

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