-
Notifications
You must be signed in to change notification settings - Fork 24
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
Use ESM build of vite #90
Conversation
Thanks for this! I think the cleaner solution is to add |
Sure. Unnecessary commits were removed and force pushed. |
@@ -1,7 +1,7 @@ | |||
import { LibraryOptions, defineConfig } from "vite"; | |||
import dts from "vite-plugin-dts"; | |||
import { isCjsPackage } from "../../scripts/esbuild-module-check.mjs"; | |||
import { packageReadmeAndPackageJson } from "../../scripts/vite-plugins"; | |||
import { packageReadmeAndPackageJson } from "../../scripts/vite-plugins.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this import fail without the extension? I would think vite
would be able to find it without trouble...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one of the things I don't quite understand. It certainly seems to work without the extension.
However, the document of vite says:
The vite.config.js file content is using the ESM syntax.
The ES module does not allow imports without extensions. At least nodejs does.
https://nodejs.org/api/esm.html#import-specifiers
Relative specifiers like './startup.js' or '../config.mjs'. They refer to a path relative to the location of the importing file. The file extension is always necessary for these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a .ts
file, I don't think we need to follow the .js
rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice, this rule remains the same for both .ts
and .js
. To ease the discomfort of importing .ts
files as .js
, TypeScript 5.0 or later allows importing in .ts
.
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#allowimportingtsextensions
(However, typescript does not change the extension in the import at compile time 😢)
Import without extension is called fake ESM (or faux ESM). I think vite.config.ts
is probably working well complemented by vite.
Code written in the fake ESM will not work in the native ESM. This is as I mentioned in the #72 .
We can write imports in vite.config.ts
without the extension, but I am not sure if this will work in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will continue to work. vite
's policy is to try to be easy to use, and I doubt they'll change that.
Please remove the .js
from the imports that were changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I dropped the changes.
Edited comments for clarity of changes. |
Thanks for the fix! |
npm test
shows following warning message.It seems that
package.json
in root does not have"type": "module"
andvite.config.ts
is used.I did following:
type: module
to package.json in rootRename allvite.config.ts
tovite.config.mts
Rename allvite.config.ts
in some files tovite.config.mts
Complete extension of relatively imported files invite.config.mts
Maybe, this is not required.