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

chore: workaround vite package build error #28

Conversation

hi-ogawa
Copy link
Member

Description

In addition to rolldown/rolldown#1642, it looks like replaceConfusingTypeNames is messing with vite package build. I haven't checked the error in a detail, but maybe it's okay to skip it for starter?

I temporary included a patch for binding.d.ts type error in the PR to verify pnpm build succeeds.

@IWANABETHATGUY
Copy link

IWANABETHATGUY commented Jul 17, 2024

The latest rolldown should be 0.12.1

@hi-ogawa
Copy link
Member Author

Thanks for the heads up! I'll test that with 0.12.1

@hi-ogawa
Copy link
Member Author

hi-ogawa commented Jul 18, 2024

Starting to check tests failure. Let me put some notes here

playground/external

EDIT: fixed by #31

before

//
// node_modules/.vite/deps/@vitejs_test-dep-that-requires.js?v=7ce498d7
//
import {
  require_vue
} from "/node_modules/.vite/deps/chunk-ACQTEB6D.js?v=d821959b";
import {
  require_slash
} from "/node_modules/.vite/deps/chunk-PNJKIXPP.js?v=d821959b";

// ../../node_modules/.pnpm/@vitejs+test-dep-that-requires@[email protected]/node_modules/@vitejs/test-dep-that-requires/index.js
var { version } = require_vue();
var slash3 = require_slash();
document.querySelector("#required-vue-version").textContent = version;
document.querySelector("#required-slash3-exists").textContent = !!slash3("foo/bar");
//# sourceMappingURL=@vitejs_test-dep-that-requires.js.map

//
// node_modules/.vite/deps/chunk-ACQTEB6D.js
//
import {
  __commonJS
} from "./chunk-PNJKIXPP.js";

// vite:cjs-external-facade:vue
import * as m from "vue";
var require_vue = __commonJS({
  "vite:cjs-external-facade:vue"(exports, module) {
    module.exports = m;
  }
});

export {
  require_vue
};

after

  • require('vue') is failing on browser.
  • rolldownCjsExternalPlugin.resolveId is not called for vue which is in external
import { require_slash_index } from "/node_modules/.vite/deps/slash_index--Dt2Pc4V.js?v=31538b99";

//#region ../../node_modules/.pnpm/@vitejs+test-dep-that-requires@[email protected]/node_modules/@vitejs/test-dep-that-requires/index.js
const { version } = require('vue');
const slash3 = require_slash_index();
document.querySelector('#required-vue-version').textContent = version;
document.querySelector('#required-slash3-exists').textContent = !!slash3('foo/bar');

//#endregion
//# sourceMappingURL=@vitejs_test-dep-that-requires.js.map

playground/dynamic-import

(EDIT: reported upstream rolldown/rolldown#1723)

before

// node_modules/.vite/deps/@vitejs_test-pkg.js
import("/home/hiroshi/code/others/vite/node_modules/.pnpm/@vitejs+test-pkg@file+playground+dynamic-import+pkg/node_modules/@vitejs/test-pkg/pkg.css");

after

// node_modules/.vite/deps/@vitejs_test-pkg.js
// not transformed
import('./pkg.css');

// then after vite import analysis rewrite
import("/node_modules/.vite/deps/pkg.css?v=b4888b04")

playground/ssr-noexternal

(EDIT: fixed by rolldown/rolldown#1655)

Error during ssr optimizeDeps. It looks like this is because of banner function. () => Promise<string> works but () => string fails.

Error: Rolldown internal error: InvalidArg, UNKNOWN_RETURN_VALUE. Cannot convert unknown to `Option<String>` in ThreadsafeFunction<RenderedChunk, Either<Either<Promise<Option<String>>, Option<String>>, UnknownReturnValue>, RenderedChunk, false>

playground/ssr-deps

Probably a same error as playground/ssr-noexternal above.


playground/tsconfig-json

Rolldown is throwing an error during deps scan since it uses esbuild's transform here but without user tsconfig which enables experimentalDecorators (cf. vitejs#15206).
See also #24 (comment).

if (loader !== 'js') {
contents = (await transform(contents, { loader })).code
}


playground/alias

(EDIT: reported upstream rolldown/rolldown#1722)

There's an error during deps optimization bundle.write.

thread 'tokio-runtime-worker' panicked at crates/rolldown/src/utils/chunk/render_chunk_exports.rs:29:53:
no entry found for key
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

It seems this alias is the problem.

// aliasing one unoptimized dep to an optimized dep
{ find: 'foo', replacement: 'vue' },

  flatIdDeps: {
    '@vue_shared': '/home/hiroshi/code/others/vite/node_modules/.pnpm/@[email protected]/node_modules/@vue/shared/dist/shared.cjs.prod.js',
    'aliased-module_index__js': '/home/hiroshi/code/others/vite/node_modules/.pnpm/@vitejs+test-aliased-module@file+playground+alias+dir+module/node_modules/@vitejs/test-aliased-module/index.js',
    vue: '/home/hiroshi/code/others/vite/node_modules/.pnpm/[email protected][email protected]/node_modules/vue/dist/vue.esm-bundler.js',
    foo: '/home/hiroshi/code/others/vite/node_modules/.pnpm/[email protected][email protected]/node_modules/vue/dist/vue.runtime.esm-bundler.js'
  },

@hi-ogawa
Copy link
Member Author

I made separate PRs to fix playground/external (#31) and fix playground/tsconfig-json (#32).

playground/ssr-noexternal and playground/ssr-deps are fixed in rolldown rolldown/rolldown#1655

For the remaining issues, I made a repro on rolldown repo: playground/dynamic-import (rolldown/rolldown#1723) and playground/alias (rolldown/rolldown#1722).

I'm closing this PR since the same change is included in other PR.

@hi-ogawa hi-ogawa closed this Jul 25, 2024
@hi-ogawa hi-ogawa deleted the chore-fix-rolldown-typecheck branch July 25, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants