fix: Include src files in vite npm bundle (for sourcemaps)

[IanVS]

Description

Fixes #3652

Bundling sourcemaps without the source causes problems, so this adds the source files to the npm bundle.

Sourcemaps were introduced in #2843.

Additional context

I wanted to try adding a conditional to the sourcemap config to only bundle them if NODE_ENV !== "production", but NODE_ENV is not set during yarn build or yarn release, so it has no effect.

Also, .npmignore files can be tricky and cause problems in some cases, although it probably could have been used here.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[patak-dev]

I am leaning towards reverting #2843 directly instead of whitelisting only .js and .d.ts files. I am afraid that this may bring issues with missing files in the package later.
If we want source maps for Vite builds for debugging, maybe the release script could set some env to avoid generating them but seems overkill for what we are gaining here.
@antfu @Shinigami92 what do you think?

[IanVS]

Also, I can add an .npmignore instead if you think that might be safer.

[Shinigami92]

Wait... one wants to have source maps and now someone else wants to exclude them? 😆 ...

Yeah we should just revert the PR or just leave them as they are, because cause they any problems? 🤷

Bundling sourcemaps without the source causes problems, so this avoids publishing them.

Which problems? Could you explain deeper?

---

For debugging or so, IMO someone could just use yarn link 🤔

[IanVS]

Which problems? Could you explain deeper?

Hi, I have details and a reproduction in the linked issue, #3652

TL;DR: when I download vite, I get sourcemaps but no source files, which is invalid and causes my tests to fail.

[Shinigami92]

TL;DR: when I download vite, I get sourcemaps but no source files, which is invalid and causes my tests to fail.

So IMO we should add these source files or revert the previous PR.
@patak-js you should decide?

[antfu]

I'd prefer to include src into the dist

[IanVS]

I'd prefer to include src into the dist

Ok, I can do that, thanks for the feedback.

[antfu]

Maybe we want to exclude the __tests__ folder

[IanVS]

Okay, then we are back to needing an .npmignore I think, if you're okay with that.

[IanVS]

Hmmmm, this isn't doing what I expect when I try npm pack. I can't seem to get files and .npmignore to play nicely together, although I thought it should be possible. Still investigating...

[IanVS]

OK, it seems that using files in package.json together with .npmignore does not work as I expected. See this old issue that was seemingly never fixed: npm/npm#4479. I've used a solution based on the last comment in that issue, and it seems to work. Here is the output of npm pack:

main

▶ npm pack --dry-run
npm notice
npm notice 📦 vite@2.3.6
npm notice === Tarball Contents ===
npm notice 121.0kB CHANGELOG.md
npm notice 212.9kB LICENSE.md
npm notice 1.2kB README.md
npm notice 2.4kB bin/openChrome.applescript
npm notice 1.5kB bin/vite.js
npm notice 4.0kB client.d.ts
npm notice 17.2kB dist/client/client.js
npm notice 30.5kB dist/client/client.js.map
npm notice 775B dist/client/env.js
npm notice 1.7kB dist/client/env.js.map
npm notice 12.1kB dist/node/chunks/dep-1dea722c.js
npm notice 29.5kB dist/node/chunks/dep-1dea722c.js.map
npm notice 18.7kB dist/node/chunks/dep-6f49f873.js
npm notice 41.2kB dist/node/chunks/dep-6f49f873.js.map
npm notice 2.2MB dist/node/chunks/dep-15ba45e1.js
npm notice 4.6MB dist/node/chunks/dep-15ba45e1.js.map
npm notice 320.0kB dist/node/chunks/dep-85bd6f48.js
npm notice 724.5kB dist/node/chunks/dep-85bd6f48.js.map
npm notice 311.9kB dist/node/chunks/dep-644cffca.js
npm notice 656.4kB dist/node/chunks/dep-644cffca.js.map
npm notice 873.4kB dist/node/chunks/dep-afb26165.js
npm notice 1.7MB dist/node/chunks/dep-afb26165.js.map
npm notice 252.0kB dist/node/chunks/dep-c511ea0e.js
npm notice 512.3kB dist/node/chunks/dep-c511ea0e.js.map
npm notice 244.3kB dist/node/cli.js
npm notice 189.4kB dist/node/cli.js.map
npm notice 82.1kB dist/node/index.d.ts
npm notice 1.2kB dist/node/index.js
npm notice 133B dist/node/index.js.map
npm notice 1.2MB dist/node/terser.js
npm notice 2.4MB dist/node/terser.js.map
npm notice 4.0kB package.json
npm notice === Tarball Details ===
npm notice name: vite
npm notice version: 2.3.6
npm notice filename: vite-2.3.6.tgz
npm notice package size: 3.3 MB
npm notice unpacked size: 16.7 MB
npm notice shasum: c7e8c25ff9edaa53b59984c64984795a93c7f176
npm notice integrity: sha512-XU71jHgADOA9o[...]dnYR9WFlYM2ew==
npm notice total files: 32
npm notice

This branch

▶ npm pack --dry-run
npm notice
npm notice 📦 vite@2.3.6
npm notice === Tarball Contents ===
npm notice 121.0kB CHANGELOG.md
npm notice 212.9kB LICENSE.md
npm notice 1.2kB README.md
npm notice 2.4kB bin/openChrome.applescript
npm notice 1.5kB bin/vite.js
npm notice 4.0kB client.d.ts
npm notice 17.2kB dist/client/client.js
npm notice 30.5kB dist/client/client.js.map
npm notice 775B dist/client/env.js
npm notice 1.7kB dist/client/env.js.map
npm notice 12.1kB dist/node/chunks/dep-1dea722c.js
npm notice 29.5kB dist/node/chunks/dep-1dea722c.js.map
npm notice 18.7kB dist/node/chunks/dep-6f49f873.js
npm notice 41.2kB dist/node/chunks/dep-6f49f873.js.map
npm notice 2.2MB dist/node/chunks/dep-15ba45e1.js
npm notice 4.6MB dist/node/chunks/dep-15ba45e1.js.map
npm notice 320.0kB dist/node/chunks/dep-85bd6f48.js
npm notice 724.5kB dist/node/chunks/dep-85bd6f48.js.map
npm notice 311.9kB dist/node/chunks/dep-644cffca.js
npm notice 656.4kB dist/node/chunks/dep-644cffca.js.map
npm notice 873.4kB dist/node/chunks/dep-afb26165.js
npm notice 1.7MB dist/node/chunks/dep-afb26165.js.map
npm notice 252.0kB dist/node/chunks/dep-c511ea0e.js
npm notice 512.3kB dist/node/chunks/dep-c511ea0e.js.map
npm notice 244.3kB dist/node/cli.js
npm notice 189.4kB dist/node/cli.js.map
npm notice 82.1kB dist/node/index.d.ts
npm notice 1.2kB dist/node/index.js
npm notice 133B dist/node/index.js.map
npm notice 1.2MB dist/node/terser.js
npm notice 2.4MB dist/node/terser.js.map
npm notice 3.9kB package.json
npm notice 13.1kB src/client/client.ts
npm notice 729B src/client/env.ts
npm notice 4.1kB src/client/overlay.ts
npm notice 239B src/client/tsconfig.json
npm notice 21.0kB src/node/build.ts
npm notice 6.6kB src/node/cli.ts
npm notice 26.9kB src/node/config.ts
npm notice 1.7kB src/node/constants.ts
npm notice 4.3kB src/node/importGlob.ts
npm notice 2.3kB src/node/index.ts
npm notice 3.9kB src/node/logger.ts
npm notice 6.4kB src/node/optimizer/esbuildDepPlugin.ts
npm notice 10.9kB src/node/optimizer/index.ts
npm notice 2.6kB src/node/optimizer/registerMissing.ts
npm notice 13.4kB src/node/optimizer/scan.ts
npm notice 5.0kB src/node/plugin.ts
npm notice 7.8kB src/node/plugins/asset.ts
npm notice 2.6kB src/node/plugins/clientInjections.ts
npm notice 33.3kB src/node/plugins/css.ts
npm notice 1.5kB src/node/plugins/dataUri.ts
npm notice 2.6kB src/node/plugins/define.ts
npm notice 5.1kB src/node/plugins/dynamicImportPolyfill.ts
npm notice 4.1kB src/node/plugins/esbuild.ts
npm notice 16.9kB src/node/plugins/html.ts
npm notice 19.3kB src/node/plugins/importAnalysis.ts
npm notice 9.0kB src/node/plugins/importAnalysisBuild.ts
npm notice 2.1kB src/node/plugins/index.ts
npm notice 2.1kB src/node/plugins/json.ts
npm notice 3.4kB src/node/plugins/manifest.ts
npm notice 556B src/node/plugins/preAlias.ts
npm notice 6.5kB src/node/plugins/reporter.ts
npm notice 21.3kB src/node/plugins/resolve.ts
npm notice 1.2kB src/node/plugins/terser.ts
npm notice 2.0kB src/node/plugins/wasm.ts
npm notice 3.0kB src/node/plugins/worker.ts
npm notice 2.0kB src/node/preview.ts
npm notice 12.1kB src/node/server/hmr.ts
npm notice 3.2kB src/node/server/http.ts
npm notice 19.6kB src/node/server/index.ts
npm notice 1.3kB src/node/server/middlewares/base.ts
npm notice 628B src/node/server/middlewares/decodeURI.ts
npm notice 2.7kB src/node/server/middlewares/error.ts
npm notice 4.9kB src/node/server/middlewares/indexHtml.ts
npm notice 3.5kB src/node/server/middlewares/proxy.ts
npm notice 4.8kB src/node/server/middlewares/static.ts
npm notice 618B src/node/server/middlewares/time.ts
npm notice 6.1kB src/node/server/middlewares/transform.ts
npm notice 6.2kB src/node/server/moduleGraph.ts
npm notice 3.2kB src/node/server/openBrowser.ts
npm notice 15.3kB src/node/server/pluginContainer.ts
npm notice 1.8kB src/node/server/searchRoot.ts
npm notice 1.3kB src/node/server/send.ts
npm notice 574B src/node/server/sourcemap.ts
npm notice 4.8kB src/node/server/transformRequest.ts
npm notice 3.8kB src/node/server/ws.ts
npm notice 3.7kB src/node/ssr/ssrExternal.ts
npm notice 1.6kB src/node/ssr/ssrManifestPlugin.ts
npm notice 4.9kB src/node/ssr/ssrModuleLoader.ts
npm notice 1.7kB src/node/ssr/ssrStacktrace.ts
npm notice 12.0kB src/node/ssr/ssrTransform.ts
npm notice 409B src/node/tsconfig.json
npm notice 13.8kB src/node/utils.ts
npm notice === Tarball Details ===
npm notice name: vite
npm notice version: 2.3.6
npm notice filename: vite-2.3.6.tgz
npm notice package size: 3.4 MB
npm notice unpacked size: 17.1 MB
npm notice shasum: 50b41d7eefbf2548279d8f13ddc62437c20b7886
npm notice integrity: sha512-4A/wDKv7klVzs[...]n8mXKbDgHQCnA==
npm notice total files: 94

[Shinigami92]

Nice, I like they way of just exclude everything and the use whitelist via .npmignore
First time seeing this workaround to using files 👍

[haoqunjiang]

files accepts glob patterns too.
https://docs.npmjs.com/cli/v7/configuring-npm/package-json#files

[IanVS]

I don't think it's possible to negate a pattern in files, though, is it? I started off using a glob to only include the types of files we wanted, but there was a (IMHO valid) concern that new files could be missed and unexpectedly wouldn't end up in the npm package.

[haoqunjiang]

It is possible:

File patterns follow a similar syntax to .gitignore, but reversed: including a file, directory, or glob pattern (*, **/*, and such) will make it so that file is included in the tarball when it's packed. Omitting the field will make it default to ["*"], which means it will include all files.

I just tested with npm pack and the output is almost identical.

  "files": [
    "bin",
    "dist",
    "client.d.ts",
    "src",
    "!/src/**/__tests__/"
  ],

[haoqunjiang]

but there was a (IMHO valid) concern that new files could be missed and unexpectedly wouldn't end up in the npm package.

Starting .npmignore with /* has the same effect.

[IanVS]

Starting .npmignore with /* has the same effect.

But then I have re-included all of dist, and src, except for excluding the tests from src. So, if a new file is added, it will be picked up, as long as it is not in __tests__.

[haoqunjiang]

But then I have re-included all of dist, and src, except for excluding the tests from src. So, if a new file is added, it will be picked up, as long as it is not in __tests__.

So it's the same as src + !/src/**/__tests__/ in files, while the latter is more intuitive IMHO.

[IanVS]

I just tested with npm pack and the output is almost identical.

I didn't realize files could be excluded in that way. I'm happy to open another PR if you'd prefer.

[haoqunjiang]

😅 I've just opened a new one #3694