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

vite.config.js behavior is different than vanilla scaffolding when importing Flickity or jQuery #64

Closed
cavellblood opened this issue Aug 19, 2021 · 49 comments
Labels

Comments

@cavellblood
Copy link

cavellblood commented Aug 19, 2021

Describe the bug

Vite in the craft-vite branch of this repo does not behave the same way when importing Flickity (or perhaps other packages as well) as it does in a new scaffolding when creating a new Vite app. I understand that it should be have a little differently because it is set up to work with Craft CMS but I would expect the import statement to behave similarly as it would with a new Vite app.

To reproduce

Steps to reproduce the behaviour:

  1. Clone the vite-craft branch of this repo.

  2. Run the following command in the buildchain directory:

    npm i flickity
  3. Then edit /src/js/app.ts by adding these two lines:

    import Flickity from "flickity";
    console.log(Flickity);

    Now the app.ts file should look like this:

     import Flickity from "flickity";
     console.log(Flickity);
     import App from "@/vue/App.vue";
     import { createApp } from "vue";
     
     // Import our CSS
     import "@/css/app.pcss";
     
     // App main
     const main = async () => {
       // Create our vue instance
       const app = createApp(App);
       // Mount the app
       const root = app.mount("#component-container");
     
       return root;
     };
     
     // Execute async function
     main().then((root) => {});
  4. Save the file and check the reloaded file contents in the browsers inspector. It should look similar to this:

    import Flickity from "/node_modules/flickity/js/index.js";
    console.log(Flickity);
    import App from "/src/vue/App.vue";
    import { createApp } from "/node_modules/vue/dist/vue.runtime.esm-bundler.js";
    import "/src/css/app.pcss";
    const main = async () => {
      const app = createApp(App);
      const root = app.mount("#component-container");
      return root;
    };
    main().then((root) => {
    });
     
    //# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJ....dfQ==
  5. Notice that in the browsers console you'll get an error like the following:

     Uncaught SyntaxError: import not found: default         app.ts:6
    

Expected behaviour

I would expect that it would behave like a new Vite scaffolding with the following steps.

  1. Run the following commands:

    npm init vite@latest my-vanila-app --template vanilla
    cd my-vanilla-app
    npm i flickity
  2. Open up the project in your IDE and edit the main.js file by adding these two lines at the top (or really anywhere in it. It all has the same effect):

    import Flickity from 'flickity'
    console.log(Flickity)

    Now the main.js file should look like this:

    import Flickity from 'flickity'
    console.log(Flickity)
    import './style.css'
    
    document.querySelector('#app').innerHTML = `
      <h1>Hello Vite!</h1>
      <a href="https://vitejs.dev/guide/features.html" target="_blank">Documentation</a>
    `
  3. Then run npm run dev

  4. Check the output of the main.js file in the network tab of the browsers inspector. It should look similar to this:

    import '/style.css'
    import __vite__cjsImport1_flickity from "/node_modules/.vite/flickity.js?v=a7f2a681"; const Flickity = __vite__cjsImport1_flickity.__esModule ? __vite__cjsImport1_flickity.default : __vite__cjsImport1_flickity
    
    console.log(Flickity)
    
    document.querySelector('#app').innerHTML = `
      <h1>Hello Vite!</h1>
      <a href="https://vitejs.dev/guide/features.html" target="_blank">Documentation</a>
    `
  5. I would expect the output of the import statement to look similar to this but it doesn't even look like it's processing it in the Craft setup:

     import __vite__cjsImport1_flickity from "/node_modules/.vite/flickity.js?v=a7f2a681"; const Flickity = __vite__cjsImport1_flickity.__esModule ? __vite__cjsImport1_flickity.default : __vite__cjsImport1_flickity

    but instead it is just like this:

    import Flickity from "/node_modules/flickity/js/index.js";
@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

I'm not sure I understand what you're saying here? Vite is Vite... what exactly isn't working?

@cavellblood
Copy link
Author

cavellblood commented Aug 19, 2021

Good question. I accidentally posted this issue before I was ready to publish it. The part that isn't working is that it's not importing the Flickity script in the craft version but it will in a new Vite scaffolding. If you do a console.log(Flickity) with the Vite scaffolding it will show this:

Screen Shot 2021-08-19 at 2 04 07 PM

@cavellblood
Copy link
Author

But with the craft setup it will show this:

Screen Shot 2021-08-19 at 2 04 59 PM

@cavellblood
Copy link
Author

I think the critical part is that with the vanilla scaffolding setup it's actually processing the file as seen here:

 import __vite__cjsImport1_flickity from "/node_modules/.vite/flickity.js?v=a7f2a681"; const Flickity = __vite__cjsImport1_flickity.__esModule ? __vite__cjsImport1_flickity.default : __vite__cjsImport1_flickity

@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

This shouldn't have anything to do with the Craft Vite plugin but rather whatever your Vite config is?

Try comparing the two vite.config.js from both projects.

@cavellblood
Copy link
Author

Right. I've tried removing as much as I can from the craft project so that it will still work and it doesn't. The vanilla project doesn't even have a vite.config.js file.

@cavellblood
Copy link
Author

I was wondering if it had something to do with it needing to interact with the back end. Perhaps this is more of a vitejs issue/question than a question for your craft setup.

@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

Can't imagine how that could ever be the case, Vite isn't even aware of the backend.

It looks to me like that in one case it's trying to do a CJS import of an old library, and in another case, it's trying to do an ESM import, and thus not finding default

@cavellblood
Copy link
Author

Well I think it is aware of the backend with the Craft setup: https://vitejs.dev/guide/backend-integration.html

build: {
    emptyOutDir: true,
    manifest: true,
    outDir: '../cms/web/dist',
    rollupOptions: {
      input: {
        main: '/src/main.js',
      },
      output: {
        sourcemap: true
      },
    }
  },

That's right about the import. I'm not sure what's causing it not to process it the same way in the Craft project.

@cavellblood
Copy link
Author

So I was wondering if something compiles differently if it has backend integration. Just a long shot. But that might be more of a question for Evan You?

@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

Compare your vite.config.js in the Craft project with the one generated by the Vite scaffolding.

@khalwat khalwat closed this as completed Aug 19, 2021
@cavellblood
Copy link
Author

cavellblood commented Aug 19, 2021

The Vite scaffolding does not create a vite.config.js. And this is the vite.config.js from the Craft project which is the same as https://github.com/nystudio107/craft/blob/craft-vite/buildchain/vite.config.js:

import vue from '@vitejs/plugin-vue'
import legacy from '@vitejs/plugin-legacy'
import ViteRestart from 'vite-plugin-restart';
import { nodeResolve } from '@rollup/plugin-node-resolve';
import critical from 'rollup-plugin-critical';
import path from 'path';

// https://vitejs.dev/config/
export default ({ command }) => ({
  base: command === 'serve' ? '' : '/dist/',
  build: {
    emptyOutDir: true,
    manifest: true,
    outDir: '../cms/web/dist',
    rollupOptions: {
      input: {
        app: '/src/js/app.ts',
      },
      output: {
        sourcemap: true
      },
    }
  },
  plugins: [
    critical({
      criticalUrl: 'https://nystudio107.com',
      criticalBase: '../cms/web/dist/criticalcss/',
      criticalPages: [
        { uri: '/', template: 'index' },
      ],
      criticalConfig: {
      }
    }),
    legacy({
      targets: ['defaults', 'not IE 11']
    }),
    nodeResolve({
      moduleDirectories: [
        path.resolve('./node_modules'),
      ],
    }),
    ViteRestart({
      reload: [
        '../src/templates/**/*',
      ],
    }),
    vue(),
    // Static Asset Fixer, see: https://github.com/vitejs/vite/issues/2394
    {
      name: 'static-asset-fixer',
      enforce: 'post',
      apply: 'serve',
      transform: (code, id) => {
        return {
          code: code.replace(/\/src\/(.*)\.(svg|jp?g|png|webp)/g, 'http://localhost:3000/src/$1.$2'),
          map: null,
        }
      },
    },
  ],
  publicDir: '../src/public',
  resolve: {
    alias: {
      '@': '/src',
    },
  },
  server: {
    host: '0.0.0.0',
  }
});

@cavellblood
Copy link
Author

I'll maybe post an issue about it over here: https://github.com/vitejs/vite/issues

@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

You have no vite.config.js in the scaffolding at all? I'm not sure I understand? Show me exactly how you're creating this Vite app?

@cavellblood
Copy link
Author

cavellblood commented Aug 19, 2021

In step 1 of the Expected Behavior of the issue I posted this:

npm init vite@latest my-vanila-app --template vanilla
cd my-vanilla-app
npm i flickity

Also you could try it with npm i jquery and get the same results.

@cavellblood
Copy link
Author

cavellblood commented Aug 19, 2021

And then try to import jQuery from 'jquery' and console.log(jQuery).

I apologize that I edited my original post so many times. That could be a cause of a lot of the confusion here. I accidentally hit the wrong keystroke and it submitted the post before I was done with it.

@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

Okay, so it's using whatever the default internal config is, then, if that file doesn't exist.

@cavellblood cavellblood changed the title vite.config.js behavior is different than vanilla scaffolding when importing Flickity vite.config.js behavior is different than vanilla scaffolding when importing Flickity or jQuery Aug 19, 2021
@cavellblood
Copy link
Author

Right.

@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

If you copy the vite.config.js from the Craft scaffolding to your newly created vanilla scaffolding, what happens (will need to update the package.json, etc. too?

@cavellblood
Copy link
Author

Good question. Hadn't tried that yet. I'll give that a try.

@whmountains
Copy link

whmountains commented Aug 19, 2021

I work with @cavellblood, here are my thoughts on the issue. (Good suggestion to test, BTW.)

It seems that the Vite build does not work with any CommonJS-only NPM package. Most packages support ES Modules, but there are a few holdouts like jQuery and flickity.

You can tell if a package supports ES modules by looking at the package.json: it should have a field like module, jsnext:main, or jsnext. This is mentioned in the Vite documentation. Originally we had the issue with Flickity, but then we went looking for a more popular CommonJS-only package and landed on jQuery.

I think the default Vite config handles this correctly, but the config in this repository somehow breaks it. We have tested creating a brand-new project from the craft-vite branch and attempting to import jQuery, but it fails in the same way.

My theory is that somehow the files are not getting processed by @rollup/plugin-commonjs, which Vite uses internally. But I have no idea why that might be.

@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

I think the default Vite config handles this correctly, but the config in this repository somehow breaks it. We have tested creating a brand-new project from the craft-vite branch and attempting to import jQuery, but it fails in the same way

This sounds right to me!

When I encounter this, I usually look for more modern, ESM packages... there is CommonJS plugin but I think this goes against the "Vite way" to some extent.

My theory is that somehow the files are not getting processed by @rollup/plugin-commonjs, which Vite uses internally. But I have no idea why that might be.

Interesting, I thought you had to explicitly use that plugin. If that's the case, then likely something in the config or in one of the plugins is causing it.

I'd start by disabling the plugins until it works.

@khalwat khalwat reopened this Aug 19, 2021
@khalwat
Copy link
Contributor

khalwat commented Aug 19, 2021

I'd be curious to hear the results here, to see if it's some config setting or some plugin that is interfering in the CommonJS loader that would normally be used.

@khalwat
Copy link
Contributor

khalwat commented Aug 20, 2021

lmk if you folks managed to figure any of this out. I may try myself as well.

@cavellblood
Copy link
Author

Will do. I plan on working on it this afternoon.

@cavellblood
Copy link
Author

Well I was able to get jQuery to work this afternoon but not Flickity. Kind of strange. Some things that I noticed that helped was to remove the leading slash everywhere this string appeared: /src/js/app.js, namely:

  • config/vite.php
  • critical-css.twig
  • Line 17 of vite.config.js

I then updated the app.js contents to this:

import $ from "jquery";

console.log($);

$("#component-container").html("It Works!");

And that worked. But I wasn't able to get Flickity working. I'll get back to this on Monday.

@cavellblood
Copy link
Author

Ok so I have some results. I just cloned down a clean version of the nystudio107/craft to verify what works with minimal changes. I just had to change one character for it to work: remove the leading slash on line 17 of vite.config.js.

export default ({ command }) => ({
  base: command === 'serve' ? '' : '/dist/',
  build: {
    emptyOutDir: true,
    manifest: true,
    outDir: '../cms/web/dist',
    rollupOptions: {
      input: {
        app: 'src/js/app.ts',  // <-- This used to be `/src/js/app.ts` 
      },
      output: {
        sourcemap: true
      },
    }
  },
  ...
}

I also had to declare the modules in the shims.d.ts or else it wouldn't work.

declare module "app";
declare module "jquery";
declare module "flickity";

I'm not super familiar with TypeScript so this was probably a beginners mistake.

@khalwat
Copy link
Contributor

khalwat commented Aug 23, 2021

Wow, that's crazy. Nice find!!

@khalwat
Copy link
Contributor

khalwat commented Aug 23, 2021

Did you also need to change it here: https://github.com/nystudio107/craft/blob/craft-vite/cms/templates/_boilerplate/_partials/critical-css.twig#L12

{{ craft.vite.script("/src/js/app.ts", false) }}

?

@cavellblood
Copy link
Author

I tried changing it there and it didn't seem to have any effect. I thought that maybe your craft-vite plugin took all that into account.

@cavellblood
Copy link
Author

It worked with the leading slash.

@cavellblood
Copy link
Author

I should note that I did all these tests when make dev was running. I didn't but should've run these tests after doing a make build to verify. I can try that now but I'm guessing it would work.

@khalwat
Copy link
Contributor

khalwat commented Aug 23, 2021

yep might as well make build and give it a whirl that way too

@cavellblood
Copy link
Author

Also, just curious about the best way to test built files. Here's what I currently do:

  • Run make dev
  • Then once all the containers are up and running I run make build in a new window.
  • When that process is complete I use Docker Desktop to stop the project_vite_1 container so that that vite server is no longer running.
  • Then I reload the page to get the built assets.

Is that the best way to test built files?

@khalwat
Copy link
Contributor

khalwat commented Aug 23, 2021

You can do it that way, or you can stop the vite container with:

docker-compose stop vite

but I typically just leave it running and do

make build

as you've suggested

@khalwat
Copy link
Contributor

khalwat commented Aug 23, 2021

That's one of the selling points of Docker-izing things, you can spin up as many instances as you need/want

@cavellblood
Copy link
Author

Ok.

What's really odd now is that when I clone down cavellblood/craft Vite doesn't seem to be working at all. It says it's running in the terminal but on the front end all I see is this:

Screen Shot 2021-08-23 at 4 51 32 PM

Maybe there's something awry with my machine. I'll try cloning down your repo to see if I have the same issue.

@cavellblood
Copy link
Author

It's not loading the app.ts file anymore.

@cavellblood
Copy link
Author

cavellblood commented Aug 23, 2021

Same thing is happening when I clone your repo down. Time to restart some things.

@cavellblood
Copy link
Author

I'm not sure what happened. I've cloned your repo down twice and I can't get the front end to load. Everything was working just fine a moment ago. I'll have to try a different computer. Sorry that this verification is taking longer than expected. I've even removed old images from Docker after quitting the project.

Anyway, I'll get back to you when I get it working.

khalwat added a commit to nystudio107/spoke-and-chain that referenced this issue Aug 23, 2021
@khalwat
Copy link
Contributor

khalwat commented Aug 23, 2021

I made you change to the Spoke and Gear P&T demo site that I converted over to Vite, and it's working as expected:

nystudio107/spoke-and-chain@b9a896c

(but it has always worked -- just noting that nothing broke)

@cavellblood
Copy link
Author

Right. Well I just was able to get things working again. I had to adjust the path in config/vite.php and criticalcss.twig to ../src/js/app.ts. I'm adding it to the PR.

@khalwat
Copy link
Contributor

khalwat commented Aug 23, 2021

Looks like the right way to do this is just change it to this in buildchain/vite.config.js:

      input: {
        app: './src/js/app.ts',
      },

...then you don't need to change anything anywhere else, at least in my testing.

khalwat added a commit that referenced this issue Aug 23, 2021
Signed-off-by: Andrew Welch <andrew@nystudio107.com>
khalwat added a commit to nystudio107/craft-plugin-vite-buildchain that referenced this issue Aug 23, 2021
@cavellblood
Copy link
Author

Ok. Good to know. I should've tried that. I'll have to test that later but I think we can close this issue now.

@khalwat
Copy link
Contributor

khalwat commented Aug 24, 2021

I'm going to do a little more testing but you've been fantastic, thank you for digging into this!

@khalwat
Copy link
Contributor

khalwat commented Aug 24, 2021

I was able to get Europa Museum ported over to Vite, thanks to this finding!

https://github.com/nystudio107/europa-museum/tree/docker-vite

@cavellblood
Copy link
Author

That's great!

So did you eventually just settle on this change:

input: {
    app: './src/js/app.ts',
},

I guess I could take a look at that repo to see what you did.

@cavellblood
Copy link
Author

I see the changes you made here: e213d4f

@khalwat
Copy link
Contributor

khalwat commented Aug 24, 2021

You got it!

davidhellmann added a commit to davidhellmann/craftcms-baukasten that referenced this issue Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants