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

Expose ImportSpecifiers on the Chunk for plugins #3374

Closed
joeljeske opened this issue Feb 7, 2020 · 9 comments · Fixed by #3722
Closed

Expose ImportSpecifiers on the Chunk for plugins #3374

joeljeske opened this issue Feb 7, 2020 · 9 comments · Fixed by #3722

Comments

@joeljeske
Copy link
Contributor

Feature Use Case

I want to write a plugin that saves all of the exports and imports of a chunk as metadata to be verified downstream for linking purposes without re-parsing the entire chunk in various module formats.

I have a multi-stage build that uses rollup for building various parts. They use import maps to glue everything back together at the end. I use typescript to ensure that the src's are importing what is available, but I want to "link" everything together before runtime and verify that all the chunks are only importing named imports that actually exist as exports on other modules.

Feature Proposal

export interface PreRenderedChunk {
    imports: string[];
+   importSpecifiers: {
+     [import: string]: string[];
+   }
}

Notes:

  • Each import key in the importSpecifiers object would exist in the imports[]
  • Each value in the string[] in the importSpecifiers object would be the named imports from that module. default could exist in this array. (I do not think * could ever appear here as I think rollup ensures it knows all the imports by this point, except in the case of re-exports? I'd love some guidance here.)
@joeljeske
Copy link
Contributor Author

@lukastaegert do you have any thoughts regarding this use case? Is there an alternative approach you might recommend instead?

@joeljeske
Copy link
Contributor Author

I am currently using this patch to expose these bits of information. I think it would be a valuable addition to the upstream. It actually seems like a missing features that chunks would expose its named exports but not the named imports.

rollup+1.26.3.patch.txt

@lukastaegert
Copy link
Member

Sorry for not moving forward on this yet, at the moment I am a little busy with Rollup@2. I will have a look shortly, it definitely looks worthwhile.

@joeljeske
Copy link
Contributor Author

@lukastaegert any thoughts on this enhancement? I think this would be valuable when using rollup as a portion of a larger build.

@lukastaegert
Copy link
Member

Fix at #3722, sorry for the wait, please have a look.

@joeljeske
Copy link
Contributor Author

Yay thanks @lukastaegert! I am wanting to try this out, but I was unable to get the package to install
npm install rollup/rollup#gh-3374-import-specifiers-in-bundle. It seemed to get me the source code but not the built dist. Is that expected?

@lukastaegert
Copy link
Member

That is weird, direct installation is working well for me. Maybe you are using some symlinks/npm link that is interfering? Are you using npm 6? Earlier versions do not support this fully.

@joeljeske
Copy link
Contributor Author

Sorry @lukastaegert, I was using yarn to install as well which was causing that trouble. Using npm instead worked.

And, all my tests pass and output comes out byte-for-byte the same. Thanks for the new feature!

One last thing, after my initial feature request, I realized that "importSpecifiers" may not be the best technically accurate name. Its more like importedImports but that is rather silly. If I understand correctly, an import specifier is the from clause. I'm not sure what the best technical name may be, but I wanted to call it out.

Anyway, thanks!!

@lukastaegert
Copy link
Member

Thanks for checking. In our abstract syntax tree, specifiers are actually kind of the correct name https://github.com/estree/estree/blob/master/es2015.md#importspecifier even though not too many people might be aware of that. In that context, the from clause is referred to as the source. importedImports is also kind of right as of course the name of the binding is specifier.imported, but I think it overlaps too much with our use of import referring to the whole import declaration. I could get behind importedBindings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants