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

"Building for Production" docs "Library Mode" section mildly broken #9147

Closed
7 tasks done
symbioquine opened this issue Jul 15, 2022 · 5 comments
Closed
7 tasks done
Labels
contribution welcome documentation Improvements or additions to documentation

Comments

@symbioquine
Copy link

symbioquine commented Jul 15, 2022

Describe the bug

Based on;

npm create vite@latest myapp --template vanilla
cd myapp

echo "import { resolve } from 'path'
import { defineConfig } from 'vite'
export default defineConfig({
  build: {
    lib: {
      entry: resolve(__dirname, 'lib/main.js'),
      name: 'MyLib',
      // the proper extensions will be added
      fileName: 'my-lib'
    },
    rollupOptions: {
      // make sure to externalize deps that shouldn't be bundled
      // into your library
      external: ['vue'],
      output: {
        // Provide global variables to use in the UMD build
        // for externalized deps
        globals: {
          vue: 'Vue'
        }
      }
    }
  }
})" > vite.config.js

mkdir lib
echo "export default function square(n) {
  return n*n;
}" > lib/main.js

echo '{
  "name": "my-lib",
  "type": "module",
  "files": ["dist"],
  "main": "./dist/my-lib.umd.cjs",
  "module": "./dist/my-lib.js",
  "exports": {
    ".": {
      "import": "./dist/my-lib.js",
      "require": "./dist/my-lib.umd.cjs"
    }
  }
}' > package.json

npm install
npm exec vite build

Expected

Library builds by copying documentation verbatim.

Actual behavior

$ npm exec vite build
failed to load config from /home/user/workspace/myapp/vite.config.js
error during build:
ReferenceError: __dirname is not defined in ES module scope
This file is being treated as an ES module because it has a '.js' file extension and '/home/user/workspace/myapp/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at file:///home/user/workspace/myapp/vite.config.js?t=1657918746154:6:22
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)
    at async loadConfigFromFile (file:///home/user/workspace/myapp/node_modules/vite/dist/node/chunks/dep-561c5231.js:62653:31)
    at async resolveConfig (file:///home/user/workspace/myapp/node_modules/vite/dist/node/chunks/dep-561c5231.js:62281:28)
    at async doBuild (file:///home/user/workspace/myapp/node_modules/vite/dist/node/chunks/dep-561c5231.js:43272:20)
    at async build (file:///home/user/workspace/myapp/node_modules/vite/dist/node/chunks/dep-561c5231.js:43261:16)
    at async CAC.<anonymous> (file:///home/user/workspace/myapp/node_modules/vite/dist/node/cli.js:747:9)

Problem

Use of __dirname in the provided vite.config.js conflicts with the subsequent recommended package.json which recommends "type": "module".

Proposed solution

Each documentation section should be internally consistent such that a new user can follow the recommendations in that section without getting sidetracked by finding solutions to problems the docs have baked in.

Here the docs should probably do one of the following:

  • Refrain from using __dirname when recommending use of "type": "module" in package.json
  • Don't recommend use of "type": "module" in package.json when using __dirname in vite.config.js
  • Include a work-around such as those listed here in vite.config.js

Related (but not duplicate) issues

Reproduction

Included in bug description

System Info

Not really relevant, but


$ node --version
v18.5.0


`vite@3.0.0`

Used Package Manager

npm

Logs

No response

Validations

@sodatea sodatea added documentation Improvements or additions to documentation contribution welcome and removed pending triage labels Jul 16, 2022
@github-actions
Copy link

Hello @symbioquine. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!

@symbioquine
Copy link
Author

Hi @sodatea, thanks for taking a look at this issue! I'd be happy to open a PR if we can agree on which of the strategies I've outlined above (copied below) is preferred - i.e. most in line with the direction the docs should be moving. (I don't have a opinion which is best.)

  • Refrain from using __dirname when recommending use of "type": "module" in package.json
  • Don't recommend use of "type": "module" in package.json when using __dirname in vite.config.js
  • Include a work-around such as those listed __dirname is not defined nodejs/help#2907 (comment) in vite.config.js

@bluwy
Copy link
Member

bluwy commented Jul 16, 2022

This should be fixed in #9121 which isn't released yet. It will use esbuild to polyfill __dirname so it works here too.

@bluwy bluwy closed this as completed Jul 16, 2022
@sodatea
Copy link
Member

sodatea commented Jul 16, 2022

🤔Though the bugs are fixed, I think we should refrain from using __dirname with type: module.
Not all environments have __dirname polyfilled. It's not a good practice anyway.

@bluwy
Copy link
Member

bluwy commented Jul 16, 2022

I agree too. We could probably make the change altogether for ts too since cts and mts are now supported, in the next major.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
contribution welcome documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants