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

Provide ESM export for @patternfly/react-styles css components. #7054

Closed
smurrayatwork opened this issue Mar 11, 2022 · 5 comments · Fixed by #7084
Closed

Provide ESM export for @patternfly/react-styles css components. #7054

smurrayatwork opened this issue Mar 11, 2022 · 5 comments · Fixed by #7084
Assignees
Milestone

Comments

@smurrayatwork
Copy link

Using bundlers that support ES Modules (such as vite) require ES Module exports. Currently, all of the component .js files under @patternfly/react-styles/css/components use require on .css files. It would be better to include in addition to this another file that uses import instead.

A nice to have in addition to this es module file would be placing the @patternfly/react-styles/css directory under both @patternfly/react-styles/dist/esm and @patternfly/react-styles/dist/js . This would allow for bundlers to easily determine which .js files to use for ES modules and commonjs modules.

@jschuler
Copy link
Collaborator

Hi @smurrayatwork, are you trying to import from the @patternfly/react-styles package directly? Or only indirectly since @patternfly/react-core depends on it?

@smurrayatwork
Copy link
Author

@jschuler ,

In this case, indirectly. I was actually able to get this working by modifying the @patternfly/react-styles package to produce .mjs files:

diff --git a/node_modules/@patternfly/react-styles/scripts/writeClassMaps.js b/node_modules/@patternfly/react-styles/scripts/writeClassMaps.js
index 39e39d8..d1d14b3 100644
--- a/node_modules/@patternfly/react-styles/scripts/writeClassMaps.js
+++ b/node_modules/@patternfly/react-styles/scripts/writeClassMaps.js
@@ -25,6 +25,16 @@ export default _default;
 `.trim()
   );
 
+const writeMJSExport = (file, classMap) =>
+  outputFileSync(
+    join(outDir, file.replace(/.css$/, '.mjs')),
+    `
+  import './${basename(file, '.css.js')}';
+  const _default = ${JSON.stringify(classMap, null, 2)};
+  export default _default;
+  `.trim()
+  );
+
 /**
  * @param {any} classMaps Map of file names to classMaps
  */
@@ -36,6 +46,7 @@ function writeClassMaps(classMaps) {
 
     writeCJSExport(outPath, classMap);
     writeDTSExport(outPath, classMap);
+    writeMJSExport(outPath, classMap);
     copyFileSync(file, join(outDir, outPath));
   });
 

The .mjs files are picked up by bundlers that recognize that extension as es modules. I haven't made a PR just yet but I will soon.

@kunyan
Copy link
Contributor

kunyan commented Mar 17, 2022

+1 here

When I use Vite or SnowPack

Will got following error

Uncaught Error: Dynamic require of "/demo-app/node_modules/@patternfly/react-styles/css/components/Backdrop/backdrop.css" is not supported
    at chunk-T4QY3MSR.js?v=40af26ba:12:9
    at node_modules/@patternfly/react-styles/css/components/Backdrop/backdrop.js (backdrop.js:3:1)
    at __require2 (chunk-T4QY3MSR.js?v=40af26ba:15:50)
    at AboutModal.tsx:4:20

@kunyan
Copy link
Contributor

kunyan commented Mar 18, 2022

I think the better way is add another code snippets for esm only

const writeESMExport = (file, classMap) =>
  outputFileSync(
    join(outDir, file.replace(/.css$/, '.mjs')),
    `
import('./${basename(file, '.css.js')}');
export default ${JSON.stringify(classMap, null, 2)};
`.trim()
  );
    writeCJSExport(outPath, classMap);
    writeDTSExport(outPath, classMap);
    writeESMExport(outPath, classMap);

@mcarrano mcarrano added this to the 2022.05 milestone Mar 25, 2022
@nicolethoen nicolethoen moved this to PR Review in PatternFly Issues Mar 25, 2022
Repository owner moved this from PR Review to Done in PatternFly Issues Apr 11, 2022
@kunyan
Copy link
Contributor

kunyan commented Apr 12, 2022

Verified with https://www.npmjs.com/package/@patternfly/react-core/v/4.202.25

@patternfly/react-core is working fine with Vite now

There has a esbuild bug in 4 hours ago evanw/esbuild#2177

vitejs/vite#7683

Please make sure you are using 0.14.34 to avoid the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants