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

Support Rollup's this.load() method in the plugin context #6810

Closed
4 tasks done
dhaig opened this issue Feb 8, 2022 · 6 comments · Fixed by #11469
Closed
4 tasks done

Support Rollup's this.load() method in the plugin context #6810

dhaig opened this issue Feb 8, 2022 · 6 comments · Fixed by #11469

Comments

@dhaig
Copy link

dhaig commented Feb 8, 2022

Clear and concise description of the problem

When a Rollup plugin needs to inspect the contents of another module, the load() method in the plugin context resolves and transforms the dependency on demand. Vite's plugin container doesn't provide it, so calling this.load() from a plugin hook throws an error. It would be nice to have it available in Vite as I've run into situations where it's necessary and as I understand it there's no workaround for the same functionality.

Suggested solution

In the plugin context, Vite can implement a version of load() that is similar to Rollup's but with a few changes:

  • the final parsing step would be skipped as Vite only parses on demand
  • the returned module information might not always have the AST available
  • it wouldn't trigger moduleParsed

In Rollup, load() returns a promise that finishes after the module is resolved, loaded, and transformed but before its dependencies have been resolved. It supports a flag resolveDependencies that makes the promise finish afterward instead. I think this option would still make sense for Vite, but I'm not 100% clear on when Vite resolves a module's dependencies.

Alternative

No response

Additional context

  • Documentation for this.load() including a few example use cases
  • I checked WMR's plugin container to see if it supports load() and it doesn't – I unfortunately couldn't find any discussion about it, so I'm not sure why they decided against it.

Validations

@luxaritas
Copy link
Contributor

Just ran into this causing an error using the latest version of rpt2 with vitest (see ezolenko/rollup-plugin-typescript2@0.33.0...0.34.0#diff-a2a171449d862fe29692ce031981047d7ab755ae7f84c707aef80701b3ea0c80R266 where 0.34 introduced a call to this.load()).

I'd be willing to help move this forward given some guidance (this goes deeper into vite than I'm currently comfortable with, so naturally my assistance is predicated on it actually being useful).

@luxaritas
Copy link
Contributor

My guess why this wasn't implemented is just that it was introduced after PluginContainer was written and wasn't noticed/picked up (this.load was added rollup 2.60, released in November '21). Got some initial pointers from Blu via Discord, so I'm starting to look into this.

@luxaritas
Copy link
Contributor

Yeah, this PR seems to lend credence to that: #5846

luxaritas added a commit to eternagame/workspace-helpers that referenced this issue Jan 5, 2023
## Summary
Updates our dependencies to the latest versions, with some exceptions

## Implementation Notes
* Lerna upgrade to v6: No relevant major changes
* Vite upgrade to v4/rollup to v3: No relevant major changes
* Cypress update to v12: No relevant major changes (but fixes HMR with
stylesheets)
* TypeScript update to v4.9: No relevant major changes
* Vitest update to v0.26: No relevant major changes
* Nx upgrade to v15: Three migrations handled, no other relevant major
changes
* Replace implicitDependencies with namedInputs + target inputs
(implicit dependencies are deprecated in favor of targetDefaults, and nx
handles package.json and nx.json itself so we can just remove our
previous implicit dependencies)
* Prefix outputs with {workspaceRoot}/{projectRoot} if needed (cypress
outputs have been changed to be relative to the project directory)
* Set project names in project.json files (we don’t use them, so nothing
to do)

Note that rollup-plugin-typescript2 has been pinned to 0.33.0. See
vitejs/vite#6810 (comment) for
more details

## Testing
Existing CI
@micha-lmxt
Copy link

My current workaround is to use server.transformRequest in dev mode:

...
   {
    ...,
    buildStart(){
        if (this.load){
            _this.load = this.load
        }
    },
    configureServer(server){
        if(!_this.load){
            _this.load = (resolvedFile) => server.transformRequest(resolvedFile.id)
        }
    }
....

@alexeyyakimovich
Copy link

#5846

Can you, please, add more info about where to add this code?

@micha-lmxt
Copy link

Sure. buildStart is a rollup/vite hook and configureServer refers to the vite specific hook. The plugin should export an object with these properties.
Example:

// Load 'file.js' early
export const plugin=({file:'file.js'})=>{
    const _this={}
    let isLoaded = false
    return {
        name:'myFileLoader',
        buildStart(){
            if (this.load){
                _this.load = this.load
            }
        },
        configureServer(server){
            if(!_this.load){
                _this.load = (resolvedFile) =>  server.transformRequest(resolvedFile.id)
            }
        },
        async resolveId(){
            if (isLoaded){
               return null;
            }
            const resolved = await this.resolve(file)
            if (resolved){
                await _this.load(resolved)
            }
        }
   
    }
}

@github-actions github-actions bot locked and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants