Skip to content

[postcss-plugin] Add unit tests#1067

Merged
necolas merged 6 commits into
facebook:mainfrom
javascripter:postcss-plugin-tests
May 19, 2025
Merged

[postcss-plugin] Add unit tests#1067
necolas merged 6 commits into
facebook:mainfrom
javascripter:postcss-plugin-tests

Conversation

@javascripter

@javascripter javascripter commented May 18, 2025

Copy link
Copy Markdown
Contributor

What changed / motivation ?

This PR adds tests for @stylexjs/postcss-plugin as per #1065 (review)

Linked PR/Issues

Fixes #1065 (review)

Additional Context

  1. Confirmed that tests pass in this branch, and if we revert the fix from the previous PR, tests fail as expected.
Screenshot 2025-05-18 at 17 34 37

Checked examples/example-nextjs still renders correctly in this branch
Screenshot 2025-05-18 at 17 36 37

Screenshots, Tests, Anything Else

Pre-flight checklist

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 18, 2025
@javascripter javascripter force-pushed the postcss-plugin-tests branch from d5cd765 to 95ef1af Compare May 18, 2025 08:45
const postcss = require('postcss');
const createBuilder = require('./builder');

module.exports = function createPlugin() {

@javascripter javascripter May 18, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builder keeps track of files internally.

I moved the code from src/index.js to src/plugin.js and made it a factory so that each test re-creates the plugin so that no state leaks over to other tests. No other change is included in this file.


async function runStylexPostcss(options = {}, inputCSS = '@stylex;') {
// Create a new instance for each test as the plugin is stateful
const stylexPostcssPlugin = createPlugin();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we re-create the plugin every time and reset internal state

Comment on lines +14 to +20
"jest": {
"testPathIgnorePatterns": [
"/node_modules/",
"/__fixtures__/"
],
"testEnvironment": "node"
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this into a jest config file (like the other packages)

@javascripter javascripter May 19, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 20592c7

"devDependencies": {
"@babel/preset-env": "^7.26.0",
"@babel/preset-react": "^7.25.3",
"jest": "^30.0.0-alpha.7",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need this as the monorepo installs jest

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ce048a6

Comment on lines +1 to +6
{
"presets": [["@babel/preset-env", { "targets": { "node": "current" } }]],
"plugins": [
["@stylexjs/babel-plugin", { "dev": false, "runtimeInjection": false }]
]
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this .babelrc.js

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4d7ec46

return result;
}

it('extracts CSS from StyleX files', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('extracts CSS from StyleX files', async () => {
test('extracts CSS from StyleX files', async () => {

Use test for all these tests please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a73a2c9

Comment on lines +30 to +31
"@babel/preset-env": "^7.26.0",
"@babel/preset-react": "^7.25.3",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ce048a6

"@babel/preset-env": "^7.26.0",
"@babel/preset-react": "^7.25.3",
"jest": "^30.0.0-alpha.7",
"react": "^18.2.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ce048a6

"is-glob": "^4.0.3"
},
"devDependencies": {
"@babel/preset-env": "^7.26.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed by jest?

@javascripter javascripter May 19, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed. I added this dependency to write tests for components that use JSX but removed it before making PR for simplicity.

Fixed in ce048a6 and 448f3a6

@necolas necolas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Good stuff

@necolas necolas merged commit d48f0c0 into facebook:main May 19, 2025
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants