- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 17
feat: rolldown vite full bundle mode #107
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
Conversation
f46540b    to
    744fe8b      
    Compare
  
    c580d0c    to
    191a670      
    Compare
  
    | commit:  | 
| '\t...typeof m === "object" && !Array.isArray(m) || typeof m === "function" ? m : {},\n' + | ||
| '\tdefault: m\n' + | ||
| '})(__vite__cjsImport0_react)', | ||
| 'const react = ((m) => m?.__esModule ? m : { ...typeof m === "object" && !Array.isArray(m) || typeof m === "function" ? m : {}, default: m })(__vite__cjsImport0_react)', | 
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 is wired for it coudle be success at rolldown-vite branch.
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's probably caused by the "vitest>vite" overrides. Vitest uses Vite to process the files and that changes the Function::toString result.
| function importModuleWithFullBundleMode() { | ||
| const importPromise = import(base + url) | ||
| return importPromise.then(() => | ||
| // @ts-expect-error globalThis.__rolldown_runtime__ | 
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 the future, I think it's good to have the types exposed from rolldown so that we can use it like:
declare var __rolldown_runtime__: RolldownHmrRuntimeThere 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.
Yeah.
| '\t...typeof m === "object" && !Array.isArray(m) || typeof m === "function" ? m : {},\n' + | ||
| '\tdefault: m\n' + | ||
| '})(__vite__cjsImport0_react)', | ||
| 'const react = ((m) => m?.__esModule ? m : { ...typeof m === "object" && !Array.isArray(m) || typeof m === "function" ? m : {}, default: m })(__vite__cjsImport0_react)', | 
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's probably caused by the "vitest>vite" overrides. Vitest uses Vite to process the files and that changes the Function::toString result.
| if (!server.httpServer) { | ||
| throw new Error('HTTP server not available') | ||
| } | 
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.
We should probably support middleware mode (it does not have server.httpServer) in the future.
657dbc6    to
    e44af73      
    Compare
  
    1d5030a    to
    24787ef      
    Compare
  
    635f53f    to
    5d68ea9      
    Compare
  
    
      
        
              This comment was marked as off-topic.
        
        
      
    
  This comment was marked as off-topic.
      
        
              This comment was marked as off-topic.
        
        
      
    
  This comment was marked as off-topic.
79e2b43    to
    6593719      
    Compare
  
    9265346    to
    bdb70a9      
    Compare
  
    2fc1ba0    to
    bfb953f      
    Compare
  
    | isBuild: config.command === 'build', | ||
| isBuild, | 
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.
Shouldn't this be instead? Does it actually need to set isBuild: false? If so, why?
isBuild: config.command === 'build',
optimizeDeps: false,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.
The full bundle mode also is build, here only add a condition with config.command === 'build' || experimental.fullBundleMode. The production mode maybe could be removed.
Here need to ensure consistency with build.
| treeshake: experimental.fullBundleMode | ||
| ? false | ||
| : options.rollupOptions.treeshake, | 
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.
Why does this have to be false?
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.
The treeshake will remove unused code, but the unused code maybe using at hmr.
| => #191 | 
Current status
Rolldown vite already integrated rolldown at dev build, could using
npx vite --fullBundleModeto preview it. It only used at client build, the ssr is not supported at now.integrated HMR features
import.meta.hot.getExports(), react plugin refPlugins
Test status
Break changes
xxx.htmlcould be seen at original vite dev, but not added it tobuild.rollupOptions.input. In full bundle mode, you need to add thebuild.rollupOptions.inputto make sure the HTML could be seen.rolldown vite test refxxx.jscould be seen at original vite dev, but it can't at full bundle mode.react plugin test reftransformIndexHtmldefaultorderispost, it will inject some code to HTML, if it need to import module, it can't work at full bundle mode. The injected code need to bundled, you need to set theordertopre. react plugin refBreak changes but could be improved
transformhook is called once at original vite. In full bundle mode, it should be once, but not implement at now, we could improve it at future. rolldown vite test reftransformIndexHtmlis running at build, theconfig.serveris invalid. react plugin ref