-
-
Notifications
You must be signed in to change notification settings - Fork 75
Migrate css-has-pseudo #21
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
Migrate css-has-pseudo #21
Conversation
…epo-migration--courageous-indian-rhinoceros-d5866b97ab
…epo-migration--courageous-indian-rhinoceros-d5866b97ab
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I've left some comments here
@@ -28,17 +27,42 @@ npm install css-has-pseudo | |||
Then include and initialize it on your document: | |||
|
|||
```js | |||
const cssHasPseudo = require('css-has-pseudo'); | |||
const cssHasPseudo = require('css-has-pseudo/dist/browser'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we streamline this with the exports
? Not a fan of requiring from dist
but can discuss too!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://nodejs.org/api/packages.html#exports-sugar
Supported since node 12, so we can do this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this, but unsure if it is fully correct.
I also don't know if unpkg will pick this up?
tricky migration
The browser script is definitely broken at this moment : https://unpkg.com/css-has-pseudo@2.0.0/browser.js
I personally think the rollup config was doing too much heavy lifting with find/replace.
This can be done by splitting correctly and preparing one dist file intended to be imported and a separate one intended for CDN's.
You will see the rollup config's are escalating further, but I hope this is the last time we need to change them.