Skip to content

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

Conversation

romainmenke
Copy link
Member

@romainmenke romainmenke commented Nov 21, 2021

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.

function trimContentForBrowser () {
	return {
		name: 'trim-content-for-browser',
		renderChunk (code) {
			const updatedCode = code
				.replace(/'use strict';\n*/, '')
				.replace(/\n*module\.exports = cssHasPseudo;/, '');

			return updatedCode;
		}
	};
}

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.

Copy link
Member

@Antonio-Laguna Antonio-Laguna left a 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');
Copy link
Member

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!

Copy link
Member Author

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!

Copy link
Member Author

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?

@Antonio-Laguna Antonio-Laguna merged commit 0bec5ff into main Nov 22, 2021
@Antonio-Laguna Antonio-Laguna deleted the css-has-pseudo--monorepo-migration--courageous-indian-rhinoceros-d5866b97ab branch November 22, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants