Repository: shipshapecode/shepherd (language: javascript)

Issue:
No longer able to exclude styles after upgrading from v12.0.4 to v12.0.5
Hi all,

After upgrading from v12.0.4 to v12.0.5, the default shepherd css styles are now included even without the use of:
`import 'shepherd.js/dist/css/shepherd.css';`

Unfortunately for our usecase where we were not looking to use the shepherd css stylesheet and instead using our own css, the recent change has caused our tour styles to break due to the high specificity of the svelte component css rules.

**expected behaviour**
I can choose whether or not I want to use the default shepherd styles by including the `shepherd.css` stylesheet.

**actual behaviour**
All shepherdjs svelte component styles are added after using `import Shepherd from 'shepherd.js'`

_shepherdjs svelte component styles included in head_
<img width="320" alt="Screenshot 2024-06-02 at 08 03 56" src="https://github.com/shepherd-pro/shepherd/assets/19910102/c9f904f7-d36d-4d3f-a126-0e29c2f743c6">

Related discussion on issue #2834.



Discussion / hints:
I wonder if this relates at all to [Vite's #4389](https://github.com/vitejs/vite/issues/4389), where CSS files are treated as side-effectual by default and aren't tree shaken correctly. The [Vue Shepherd](https://github.com/shepherd-pro/vue-shepherd) project uses Webpack instead of Vite for bundling its E2E tests and I just tested it: not including an explicit import does *not* include the default CSS rules.

Might be worth spooling up a couple of skeleton projects for the two bundlers to clean room test it. If it's Vite-specific, it'll probably still need to be fixed (being one of the largest bundlers used), but at least that gives us an idea for *why* this suddenly happened.

Though I couldn't say why Vite wouldn't have been doing this before with the wildcard, it may be possible to leverage that as a fix by using a wildcard for this again (like `/dist/css/*`)?
@Kenneth-Sills I think a wildcard might be a good fix here. Any way we can confirm easily or should we just try it?
@RobbieTheWagner I'd always err on the side of caution and try to confirm things first, personally. 😅 

So I created a totally fresh Vite project and took a look at what's going on. It's not *all* the styles being imported by default, it's just the styles for any Svelte component being added to the import graph (my demo only had the modal styles, for instance). That immediately excluded my prior guess that it was related to the Vite bug.

So I took a closer look at [our earlier PR](https://github.com/shepherd-pro/shepherd/pull/2839/files#diff-649a876c7a9ec275393ebd19e00765b0cd08d924aa3bd405a0f810c4327fe687) and I think this comes down to the Rollup config changes we made. From the [Rollup Svelte Plugin README](https://github.com/sveltejs/rollup-plugin-svelte?tab=readme-ov-file#extracting-css):

> By default (when `emitCss: true`) the CSS styles will be emitted into a virtual file...
 If you set `emitCss: false` and your Svelte components contain `<style>` tags, the compiler will add JavaScript that injects those styles into the page when the component is rendered. That's not the default, because it adds weight to your JavaScript, prevents styles from being fetched in parallel with your code, and can even cause CSP violations.

Our first approach involved creating a separate build for the CSS bundle and disabling that `emitCss` flag for the JS ones, but that's not *quite* correct given the behavior described above. Instead, we should re-enable `emitCss` with the `postcss` plugin for the ESM/CJS builds, delete the isolated CSS build, and then in `package.json` we can use `exports` to map to remap the CSS file:

```json
    "./dist/css/*": {
      "import": "./dist/esm/css/*",
      "require": "./dist/cjs/css/*"
    },
```

*Wildcard being used above, since that's just what I had tested with.*

If we were so inclined, we could also throw in a copy operation to the ESM build to output an extra to `./dist/css/*`, in case anyone is using old tooling that doesn't support the `export` remapping, though I don't *think* that's necessary?

---

I've tested the above in my Vite demo project and it seems to work!

I haven't had a chance to test it with a Webpack build though, nor have I run the E2E tests for this project against those changes. It's getting quite late, so that's something I'd have to tackle tomorrow.

Anyway, let me know if you'd like to pick this up. If you're busy, I'd be happy to look at this again tomorrow night and try to get the behavior confirmed and a PR submitted before I'm out of commission Thursday.

Thanks!
@Kenneth-Sills we really appreciate this work and would absolutely welcome a PR if you're able. It would be great to do another release ASAP to help folks who don't want these extra styles and restore previous workflows.

Fix patch:
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 9481077e2..80b712742 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -240,9 +240,6 @@ importers:
       cssnano:
         specifier: ^7.0.1
         version: 7.0.1(postcss@8.4.38)
-      del:
-        specifier: ^7.1.0
-        version: 7.1.0
       eslint-plugin-svelte:
         specifier: ^2.39.0
         version: 2.39.0(eslint@8.57.0)(svelte@4.2.15)
@@ -3348,7 +3345,7 @@ packages:
     engines: {node: ^16.14.0 || >=18.0.0}
     dependencies:
       '@npmcli/git': 5.0.6
-      glob: 10.3.12
+      glob: 10.3.15
       hosted-git-info: 7.0.1
       json-parse-even-better-errors: 3.0.1
       normalize-package-data: 6.0.0
@@ -8518,18 +8515,6 @@ packages:
     dependencies:
       is-glob: 4.0.3
 
-  /glob@10.3.12:
-    resolution: {integrity: sha512-TCNv8vJ+xz4QiqTpfOJA7HvYv+tNIRHKfUWw/q+v2jdgN4ebz+KY9tGx5J4rHP0o84mNP+ApH66HRX8us3Khqg==}
-    engines: {node: '>=16 || 14 >=14.17'}
-    hasBin: true
-    dependencies:
-      foreground-child: 3.1.1
-      jackspeak: 2.3.6
-      minimatch: 9.0.4
-      minipass: 7.0.4
-      path-scurry: 1.10.2
-    dev: true
-
   /glob@10.3.15:
     resolution: {integrity: sha512-0c6RlJt1TICLyvJYIApxb8GsXoai0KUP7AxKKAtsYXdgJR1mGEUa7DgwShbdk1nly0PYoZj01xd4hzbq3fsjpw==}
     engines: {node: '>=16 || 14 >=14.18'}
@@ -8543,6 +8528,7 @@ packages:
 
   /glob@7.2.3:
     resolution: {integrity: sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==}
+    deprecated: Glob versions prior to v9 are no longer supported
     dependencies:
       fs.realpath: 1.0.0
       inflight: 1.0.6
@@ -8555,6 +8541,7 @@ packages:
   /glob@8.1.0:
     resolution: {integrity: sha512-r8hpEjiQEYlF2QU0df3dS+nxxSIreXQS1qRhMJM0Q5NDdR386C7jb7Hwwod8Fgiuex+k0GFjgft18yvxm5XoCQ==}
     engines: {node: '>=12'}
+    deprecated: Glob versions prior to v9 are no longer supported
     dependencies:
       fs.realpath: 1.0.0
       inflight: 1.0.6
@@ -9293,6 +9280,7 @@ packages:
 
   /inflight@1.0.6:
     resolution: {integrity: sha512-k92I/b08q4wvFscXCLvqfsHCrjrF7yiXsQuIVvVE7N82W3+aqpzuUdBbfhWcy/FZR3/4IgflMgKLOsvPDrGCJA==}
+    deprecated: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
     dependencies:
       once: 1.4.0
       wrappy: 1.0.2
@@ -12804,14 +12792,6 @@ packages:
       path-root-regex: 0.1.2
     dev: true
 
-  /path-scurry@1.10.2:
-    resolution: {integrity: sha512-7xTavNy5RQXnsjANvVvMkEjvloOinkAjv/Z6Ildz9v2RinZ4SBKTWFOVRbaF8p0vpHnyjV/UwNDdKuUv6M5qcA==}
-    engines: {node: '>=16 || 14 >=14.17'}
-    dependencies:
-      lru-cache: 10.2.0
-      minipass: 7.0.4
-    dev: true
-
   /path-scurry@1.11.1:
     resolution: {integrity: sha512-Xa4Nw17FS9ApQFJ9umLiJS4orGjm7ZzwUrwamcGQuHSzDyth9boKDaycYdDcZDuqYATXw4HFXgaqWTctW/v1HA==}
     engines: {node: '>=16 || 14 >=14.18'}
@@ -14743,6 +14723,7 @@ packages:
 
   /rimraf@2.7.1:
     resolution: {integrity: sha512-uWjbaKIK3T1OSVptzX7Nl6PvQ3qAGtKEtVRjRuazjfL3Bx5eI409VZSqgND+4UNnmzLVdPj9FqFJNPqBZFve4w==}
+    deprecated: Rimraf versions prior to v4 are no longer supported
     hasBin: true
     dependencies:
       glob: 7.2.3
@@ -14750,6 +14731,7 @@ packages:
 
   /rimraf@3.0.2:
     resolution: {integrity: sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==}
+    deprecated: Rimraf versions prior to v4 are no longer supported
     hasBin: true
     dependencies:
       glob: 7.2.3
diff --git a/shepherd.js/package.json b/shepherd.js/package.json
index 5c822f9d3..6d2a9f01e 100644
--- a/shepherd.js/package.json
+++ b/shepherd.js/package.json
@@ -31,10 +31,7 @@
         "default": "./dist/cjs/shepherd.cjs"
       }
     },
-    "./dist/css/shepherd.css": {
-      "import": "./dist/css/shepherd.css",
-      "require": "./dist/css/shepherd.css"
-    },
+    "./dist/css/shepherd.css": "./dist/css/shepherd.css",
     "./*": {
       "import": {
         "types": "./dist/esm/*.d.ts",
@@ -80,7 +77,6 @@
     "@rollup/plugin-replace": "^5.0.5",
     "autoprefixer": "^10.4.19",
     "cssnano": "^7.0.1",
-    "del": "^7.1.0",
     "eslint-plugin-svelte": "^2.39.0",
     "execa": "^9.1.0",
     "postcss": "^8.4.38",
diff --git a/shepherd.js/rollup.config.mjs b/shepherd.js/rollup.config.mjs
index f000f2fb0..2aac2646a 100644
--- a/shepherd.js/rollup.config.mjs
+++ b/shepherd.js/rollup.config.mjs
@@ -30,7 +30,7 @@ const plugins = [
     preprocess: sveltePreprocess({
       typescript: true
     }),
-    emitCss: false
+    emitCss: true
   }),
   nodeResolve({
     browser: true,
@@ -44,6 +44,10 @@ const plugins = [
   babel({
     extensions: ['.cjs', '.js', '.ts', '.mjs', '.html', '.svelte']
   }),
+  postcss({
+    plugins: [autoprefixer, cssnanoPlugin],
+    extract: 'css/shepherd.css'
+  }),
   license({
     banner
   }),
@@ -60,41 +64,6 @@ if (process.env.DEVELOPMENT) {
 }
 
 export default [
-  // This first build is just to generate the CSS
-  {
-    input: 'src/shepherd.ts',
-
-    output: {
-      dir: 'dist',
-      entryFileNames: '[name].mjs',
-      format: 'es',
-      sourcemap: true
-    },
-    plugins: [
-      svelte({
-        preprocess: sveltePreprocess({
-          typescript: true
-        }),
-        emitCss: true
-      }),
-      nodeResolve({
-        browser: true,
-        exportConditions: ['svelte'],
-        extensions: ['.js', '.json', '.mjs', '.svelte', '.ts'],
-        modulesOnly: true
-      }),
-      replace({
-        'process.env.NODE_ENV': JSON.stringify(env)
-      }),
-      babel({
-        extensions: ['.cjs', '.js', '.ts', '.mjs', '.html', '.svelte']
-      }),
-      postcss({
-        plugins: [autoprefixer, cssnanoPlugin],
-        extract: 'css/shepherd.css'
-      })
-    ]
-  },
   {
     input: 'src/shepherd.ts',
 
@@ -173,7 +142,7 @@ export default [
       }),
       ...plugins,
       {
-        name: 'Build Declarations',
+        name: 'Fix CJS exports',
         closeBundle: async () => {
           console.log('Fix CJS export default -> export =');
 
@@ -189,6 +158,29 @@ export default [
           );
           fs.writeFileSync(declarationFile, content);
         }
+      },
+      {
+        name: 'Fix CSS',
+        closeBundle: async () => {
+          console.log('Copying CSS to the root');
+
+          await execaCommand(`mkdir ./dist/css`, {
+            stdio: 'inherit'
+          });
+
+          await execaCommand(
+            `cp ./dist/esm/css/shepherd.css ./dist/css/shepherd.css`,
+            {
+              stdio: 'inherit'
+            }
+          );
+
+          console.log('Deleting extra CSS files');
+
+          await execaCommand(`rimraf ./dist/esm/css ./dist/cjs/css`, {
+            stdio: 'inherit'
+          });
+        }
       }
     ]
   }


Test patch:
diff --git a/test/cypress/examples/css/no-css-import.html b/test/cypress/examples/css/no-css-import.html
new file mode 100644
index 000000000..5bde486ac
--- /dev/null
+++ b/test/cypress/examples/css/no-css-import.html
@@ -0,0 +1,16 @@
+<html>
+  <head>
+    <script type="module">
+      import Shepherd from '../../node_modules/shepherd.js/dist/esm/shepherd.mjs';
+      window.Shepherd = Shepherd;
+    </script>
+  </head>
+
+  <body>
+    <div
+      data-test-id="fake-modal-overlay"
+      class="shepherd-modal-overlay-container"
+    >
+    </div>
+  </body>
+</html>
diff --git a/test/cypress/examples/css/with-css-import.html b/test/cypress/examples/css/with-css-import.html
new file mode 100644
index 000000000..72b0cf692
--- /dev/null
+++ b/test/cypress/examples/css/with-css-import.html
@@ -0,0 +1,20 @@
+<html>
+  <head>
+    <link
+      rel="stylesheet"
+      href="../../node_modules/shepherd.js/dist/css/shepherd.css"
+    />
+    <script type="module">
+      import Shepherd from '../../node_modules/shepherd.js/dist/esm/shepherd.mjs';
+      window.Shepherd = Shepherd;
+    </script>
+  </head>
+
+  <body>
+    <div
+      data-test-id="fake-modal-overlay"
+      class="shepherd-modal-overlay-container"
+    >
+    </div>
+  </body>
+</html>
diff --git a/test/cypress/integration/css.cy.js b/test/cypress/integration/css.cy.js
new file mode 100644
index 000000000..9fa71251f
--- /dev/null
+++ b/test/cypress/integration/css.cy.js
@@ -0,0 +1,20 @@
+/*
+ * The `.shepherd-modal-overlay-container` class is chosen for inspection
+ * because the Svelte styles for the modal are added to the import graph as
+ * soon as Shepherd's JS is imported at all:
+ *
+ * shepherd.ts -> tour.ts -> components/shepherd-modal.svelte
+ */
+describe('CSS Import Behavior', () => {
+  it('includes project styles when explicitly imported', () => {
+    cy.visit('/examples/css/with-css-import');
+    cy.window().should('have.property', 'Shepherd');
+    cy.get('[data-test-id="fake-modal-overlay"]').should('have.css', 'pointer-events', 'none');
+  });
+
+  it('DOES NOT include project styles without explicit import', () => {
+    cy.visit('/examples/css/no-css-import');
+    cy.window().should('have.property', 'Shepherd');
+    cy.get('[data-test-id="fake-modal-overlay"]').should('not.have.css', 'pointer-events', 'none');
+  });
+});