[1.0] Drop CJS build and export types first

[benmccann]

Closes #234. There were only four projects I saw using CJS and I've sent PRs to migrate three of them

Also, types must be exported first. See https://publint.dev/laravel-vite-plugin@0.7.8 for more details regarding that

[timacdonald]

Hey Ben,

Thank you for this.

Is Vite removing support for CJS in a major version?

I just spun up an older version of the Laravel application skeleton and confirmed that this change alone will break potentially thousands(?) of user-land Laravel applications that were created before we moved the skeleton (https://github.com/laravel/laravel) to use "type": "modules".

The Vite configuration file for the Laravel skeleton has always used ESM import / export...

import { defineConfig } from 'vite';
import laravel from 'laravel-vite-plugin';

export default defineConfig({
    plugins: [
        laravel({
            input: ['resources/css/app.css', 'resources/js/app.js'],
            refresh: true,
        }),
    ],
});

However, because we didn't have "type": "module" in the package.json we it was looking for the CJS build version of the plugin.

After we migrated the skeleton to use "type": "module" we continued to build the CJS version alongside the ESM version. We also build the plugin with esbuild, not Vite:

vite-plugin/package.json

Lines 38 to 39 in f89fcde

	"build-plugin-cjs": "esbuild src/index.ts --platform=node --format=cjs --outfile=dist/index.cjs --define:import.meta.url=import_meta_url --inject:./import.meta.url-polyfill.js",
	"build-plugin-esm": "esbuild src/index.ts --platform=node --format=esm --outfile=dist/index.mjs",

Remembering I'm not an expert on the JS build tooling, I think that Vite dropping support for CJS shouldn't impact this library directly, as we don't build the library with Vite - although it would mean we could drop CJS support, which is what your PR is aiming to do.

It would mean that any user-land application that wanted to upgrade to the version of Vite that drops CJS support would need to make sure they have "type": "module" in the projects package.json

I would love to know if that sounds correct to you.

[benmccann]

I just spun up an older version of the Laravel application skeleton and confirmed that this change alone will break potentially thousands(?) of user-land Laravel applications that were created before we moved the skeleton (https://github.com/laravel/laravel) to use "type": "modules".

Did you test or are you inferring that from the code? If you tested it, can you share the steps with me so I can check it out? Can you share a link to one of these old projects with me?

Is Vite removing support for CJS in a major version?

Yes, it would be in a major version. TBD whether Vite 5 will remove it or just cause a warning, but either way we're trying to migrate as much as we can now.

However, because we didn't have "type": "module" in the package.json we it was looking for the CJS build version of the plugin.

I wouldn't expect that to be true. If you're doing an import it should look up the ESM version in the exports map unless there's a build step that's converting the config file from ESM to CJS

It would mean that any user-land application that wanted to upgrade to the version of Vite that drops CJS support would need to make sure they have "type": "module" in the projects package.json

No, they shouldn't need to go that far. They just need to use the plugin from an .mjs config file. Converting just the one file is generally much easier than converting the whole project. I only found four projects that even needed to be updated: #234 (comment)

[timacdonald]

Did you test or are you inferring that from the code? If you tested it, can you share the steps with me so I can check it out? Can you share a link to one of these old projects with me?

Here is a bash script that produces the problem. It downloads the skeleton before we made the "type": "modules" in the package.json the default for new projects.

This is the commit before we made the change: https://github.com/laravel/laravel/tree/d14bdeeb6db121cc7d181ce5497211612ce4bc10 if you don't feel comfortable running the wget unzip etc.

You will need to have the laravel-vite-plugin locally built with your branch. There is an npm run build script. You will then need to have it linked with npm link.

wget --output-document laravel-skeleton-pre-modules.zip https://github.com/laravel/laravel/archive/d14bdeeb6db121cc7d181ce5497211612ce4bc10.zip
unzip ./laravel-skeleton-pre-modules.zip -d ./
cd laravel-d14bdeeb6db121cc7d181ce5497211612ce4bc10
npm install
npm link laravel-vite-plugin
npm run build

Yes, it would be in a major version. TBD whether Vite 5 will remove it or just cause a warning, but either way we're trying to migrate as much as we can now.

Good to hear there will be a major.

I wouldn't expect that to be true. If you're doing an import it should look up the ESM version in the exports map unless there's a build step that's converting the config file from ESM to CJS

We just have the vite.config.js as plain ESM without anything fancy happening.

This is the error...

This error is resolved if I add "type": "module" into the projects package.json and the build is successful.

[benmccann]

Hmm. The skeleton project doesn't look valid to me because it has ESM code in a .js file without "type": "module". When you don't have "type": "module" any .js file will be interpreted as CJS. The fix is either to add "type": "module" as you noted or to rename the file to use the .mjs extension, which is generally the much easier thing to do.

I'll let you figure out how to proceed and take it from here. One suggestion would be to push a release that does a check for export default in vite.config.js and the absence of "type": "module" in package.json. If those conditions are both true then you could suggest to the user that they need to rename their config file to vite.config.mjs or better yet take the long-term but more expensive fix of adding "type": "module" and converting their whole project to ESM.

[timacdonald]

Something to keep in mind is that any change to the Laravel skeleton will not roll out to existing applications, as that code all becomes user-land code.

I think this will actually be alright, though - although not ideal.

When Vite 5 is released we would tag version a major version of the plugin.

We would note in our upgrade guide that Vite 5 is now required and that "type": "module" should be placed in the package.json file.

So I think we can probably move ahead with this as it is. I'm going to discuss it with the team today internally.

Going to mark this one as a draft until we know how we want to proceed.

Thanks, Ben!

[timacdonald]

Happy to get this one moving into our yet to be released 1.x branch.

This 1.x branch won't be released / tagged until Vite 5 comes out.

We will document the upgrade path in the upgrade guide once Vite 5 is released and we have an understanding of any additional things we need to document for Laravel projects.

Thanks again, Ben!