frenic / csstype

Strict TypeScript and Flow types for style based on MDN data
MIT License
1.73k stars 72 forks source link

Error: The package may have incorrect main/module/exports #158

Closed luigiMinardi closed 2 years ago

luigiMinardi commented 2 years ago

Description

The package.json of the csstype project is not declaring the main file and that is causing an error with vite.

The error appears to be a conflict since it doesn't appear unless you add some other packages, but the problem is caused in just this package (if you're using it (has it exported in your code)) and can be solved by declaring this main file of the package in the package.json.

Screenshots

Screenshot_2022-05-09_15-22-37 Screenshot_2022-05-09_15-22-59

To Reproduce

Before starting the reproduction, the csstype version of the reproduction is 2.6.20, and the project dependencies created will be with this versions:

  "dependencies": {
    "@vueuse/core": "^8.4.2",
    "vue": "^3.2.25"
  },
  "devDependencies": {
    "@vitejs/plugin-vue": "^2.3.1",
    "typescript": "^4.5.4",
    "vite": "^2.9.7",
    "vue-tsc": "^0.34.7"
  }

Reproducing:

Create the project that has the csstype dependence and add a package that can cause a confict. In the terminal, run:

yarn create vite testing --template vue-ts && cd testing && yarn add @vueuse/core

Import the csstype:

Open the project (code . if you use VSC).

At src/App.vue add import { Color } from 'csstype'; at the <script setup lang="ts">. It will be like that:

<script setup lang="ts">
// This starter template is using Vue 3 <script setup> SFCs
// Check out https://vuejs.org/api/sfc-script-setup.html#script-setup
import HelloWorld from './components/HelloWorld.vue'
import { Color } from 'csstype';
</script>

Getting the error:

Try to run the project in your terminal:

yarn dev

You'll get the error:

yarn run v1.22.17
$ vite

  vite v2.9.8 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  ready in 188ms.

✘ [ERROR] [plugin vite:dep-scan] Failed to resolve entry for package "csstype". The package may have incorrect main/module/exports specified in its package.json: Failed to resolve entry for package "csstype". The package may have incorrect main/module/exports specified in its package.json.
...

Validations

Fixing the bug

As said by the vite, The package may have incorrect main/module/exports specified in its package.json, the fix is therefore simple, its needed to add a primary entry point to the program. I fixed it and submitted a PR. Hope that it's pulled soon or solved in any other possible way.

luigiMinardi commented 2 years ago

157 Is also an option to fix the bug, although I don't have knowledge to say which of them is better in long-term, maybe both (?).

luigiMinardi commented 2 years ago

For those who can't wait the fix, go to node_modules/csstype/package.json and change the main and types to this:

    "main": "./index.js.flow",
    "types": "./index.d.ts",
meyer commented 2 years ago

@luigiMinardi out of curiosity, does changing import { Color } to import type { Color } change anything related to this error?

luigiMinardi commented 2 years ago

does changing import { Color } to import type { Color } change anything related to this error?

@meyer Yes, if I add import type { Color } it works, and for some reason if I remove the type after it, it keep working, even if I restart the machine, I imagine that it has something to do with the cache isn't it?

Anyway, can you tell me why with the import type it works and without it doesn't? Is it a good practice in typescript? I'm a pretty newbie on it so i'll be so thankful if you can answer those questions.

Also, deducing that the type will mean that everything imported there will be a type, when importing from a source that has types and functions for example, will the functions work being imported in the same line or will I need two lines importing from the same place? (Not a problem though, asking just to understand how it works.)

meyer commented 2 years ago

@meyer Yes, if I add import type { Color } it works, and for some reason if I remove the type after it, it keep working, even if I restart the machine, I imagine that it has something to do with the cache isn't it?

@luigiMinardi This is a bit scary but yeah, sounds like a caching problem.

Anyway, can you tell me why with the import type it works and without it doesn't? Is it a good practice in typescript? I'm a pretty newbie on it so i'll be so thankful if you can answer those questions.

I’m not familiar with the internals of vite, but if they deal with imports the way most folks deal with imports, they’re only scanning dependencies with imports that have runtime impact. import type statements can be completely stripped during the compile process since they don’t import code that makes it into the runtime. My guess is that vite checks for type imports when its parsing your code and skips over them.

Also, deducing that the type will mean that everything imported there will be a type, when importing from a source that has types and functions for example, will the functions work being imported in the same line or will I need two lines importing from the same place? (Not a problem though, asking just to understand how it works.)

For importing types and non-types from the same source, TypeScript lets you do the following:

import { RuntimeThing, type TypeOnlyThing } from 'some-module';

The type keyword is not essential. Its primary function is to hint to the compiler that there’s no runtime code associated with the import. This is important in cases where import statements cause side effects. A bundler that’s being extra safe will turn this:

import { ExampleType } from 'example-module';

into this:

import 'example-module';

just in case importing that module causes side effects that still need to happen (for example, adding some CSS to the DOM or creating a singleton).

The same bundler will completely remove this import statement:

import type { ExampleType } from 'example-module';
meyer commented 2 years ago

Ahha, it’s even simpler than I thought. dep-scan uses a regex to extract import statements and the regex explicitly excludes import followed by type: https://github.com/vitejs/vite/blob/330e0a9/packages/vite/src/node/optimizer/scan.ts#L36-L45

luigiMinardi commented 2 years ago

Ahha, it’s even simpler than I thought. dep-scan uses a regex to extract import statements and the regex explicitly excludes import followed by type: https://github.com/vitejs/vite/blob/330e0a9/packages/vite/src/node/optimizer/scan.ts#L36-L45

So, lets see if I understood right. The bundler will ignore the import followed by type and that is why you can bundle the app even with the package being buggy, is it right?

frenic commented 2 years ago

It seems like it according to the regex. Did it work?

luigiMinardi commented 2 years ago

It seems like it according to the regex. Did it work?

@frenic yes, adding type make it work again. Even so, looking at other npm packages, isn't declaring a main file to the package a good thing?

meyer commented 2 years ago

So, lets see if I understood right. The bundler will ignore the import followed by type and that is why you can bundle the app even with the package being buggy, is it right?

@luigiMinardi Not a bug on the csstype side of things. Because this is a type-only package, there is no JS to link to in a main field. For example, @types/react provides a main value of "" as well (link). vite’s dep-scan command should probably ignore packages with a blank main value. It’s a missing check on their end.

luigiMinardi commented 2 years ago

So, lets see if I understood right. The bundler will ignore the import followed by type and that is why you can bundle the app even with the package being buggy, is it right?

@luigiMinardi Not a bug on the csstype side of things. Because this is a type-only package, there is no JS to link to in a main field. For example, @types/react provides a main value of "" as well (link). vite’s dep-scan command should probably ignore packages with a blank main value. It’s a missing check on their end.

Ooh, understood, thanks for the info. I'll close the issue since everything was done on the csstype side :)