-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Expose Graph information for pluginContext calls #2565
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
f235f0f
Expose graph module information for plugins.
samccone be852fe
Refactor plugin module information and add test
lukastaegert 8ca2d8e
Add test that early access to moduleIds and module information is han…
lukastaegert 63f1229
Merge branch 'master' into sjs/dump_graph
lukastaegert File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { version as rollupVersion } from 'package.json'; | ||
| import Graph from '../Graph'; | ||
| import Module from '../Module'; | ||
| import { | ||
| InputOptions, | ||
| Plugin, | ||
|
|
@@ -114,6 +115,21 @@ export function createPluginDriver( | |
| warning.plugin = plugin.name || '(anonymous plugin)'; | ||
| graph.warn(warning); | ||
| }, | ||
| moduleIds: graph.moduleById.keys(), | ||
| getModuleInfo: (moduleId: string) => { | ||
| const foundModule = graph.moduleById.get(moduleId); | ||
| if (foundModule == null) { | ||
| throw new Error(`Unable to find module ${moduleId}`); | ||
| } | ||
|
|
||
| return { | ||
| id: foundModule.id, | ||
| isExternal: !!foundModule.isExternal, | ||
| importedIds: foundModule.isExternal | ||
| ? [] | ||
| : (foundModule as Module).sources.map(id => (foundModule as Module).resolvedIds[id]) | ||
| }; | ||
| }, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very nice, yes, that is indeed very maintainable now!
|
||
| watcher | ||
| }; | ||
| return context; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
26 changes: 26 additions & 0 deletions
26
test/function/samples/plugin-module-information-early-access/_config.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| const assert = require('assert'); | ||
| const path = require('path'); | ||
|
|
||
| const ID_MAIN = path.join(__dirname, 'main.js'); | ||
|
|
||
| module.exports = { | ||
| description: 'handles accessing module information via plugins early in a graceful way', | ||
| options: { | ||
| external: ['path'], | ||
| plugins: [ | ||
| { | ||
| buildStart() { | ||
| assert.deepEqual(Array.from(this.moduleIds), []); | ||
| // should throw "not found" error | ||
| this.getModuleInfo(ID_MAIN); | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| error: { | ||
| code: 'PLUGIN_ERROR', | ||
| hook: 'buildStart', | ||
| message: `Unable to find module ${ID_MAIN}`, | ||
| plugin: 'Plugin at pos 0' | ||
| } | ||
| }; |
1 change: 1 addition & 0 deletions
1
test/function/samples/plugin-module-information-early-access/main.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export default 42; |
47 changes: 47 additions & 0 deletions
47
test/function/samples/plugin-module-information/_config.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| const assert = require('assert'); | ||
| const path = require('path'); | ||
|
|
||
| const ID_MAIN = path.join(__dirname, 'main.js'); | ||
| const ID_FOO = path.join(__dirname, 'foo.js'); | ||
| const ID_NESTED = path.join(__dirname, 'nested', 'nested.js'); | ||
| const ID_PATH = 'path'; | ||
|
|
||
| let rendered = false; | ||
|
|
||
| module.exports = { | ||
| description: 'provides module information on the plugin context', | ||
| options: { | ||
| external: ['path'], | ||
| plugins: [ | ||
| { | ||
| renderStart() { | ||
| rendered = true; | ||
| assert.deepEqual(Array.from(this.moduleIds), [ID_MAIN, ID_FOO, ID_NESTED, ID_PATH]); | ||
| assert.deepEqual(this.getModuleInfo(ID_MAIN), { | ||
| id: ID_MAIN, | ||
| importedIds: [ID_FOO, ID_NESTED], | ||
| isExternal: false | ||
| }); | ||
| assert.deepEqual(this.getModuleInfo(ID_FOO), { | ||
| id: ID_FOO, | ||
| importedIds: [ID_PATH], | ||
| isExternal: false | ||
| }); | ||
| assert.deepEqual(this.getModuleInfo(ID_NESTED), { | ||
| id: ID_NESTED, | ||
| importedIds: [ID_FOO], | ||
| isExternal: false | ||
| }); | ||
| assert.deepEqual(this.getModuleInfo(ID_PATH), { | ||
| id: ID_PATH, | ||
| importedIds: [], | ||
| isExternal: true | ||
| }); | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| bundle() { | ||
| assert.ok(rendered); | ||
| } | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import path from 'path'; | ||
|
|
||
| export const foo = path.resolve('foo'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export {foo} from './foo.js'; | ||
| import { nested } from './nested/nested'; | ||
|
|
||
| export {nested}; |
3 changes: 3 additions & 0 deletions
3
test/function/samples/plugin-module-information/nested/nested.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import { foo } from '../foo.js'; | ||
|
|
||
| export const nested = 'nested' + foo; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What happens if this is called before the module has been resolved? Is that possible?
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.
For now,
moduleIdswill be empty andgetModuleInfowill throw a "module not found" error. Added a test to make sure this is the case. This may or may not be what users want but if we e.g. do not provide this information on certain hooks at all, I fear this is something that is hard to maintain properly when e.g. new hooks are added. Thus here you get a snapshot of what the current state of the graph is.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.
Thanks for clarifying. It might be nice to separate the graph into two states then - resolved and unresolved, and for this function to always throw an early error when called when the graph is partially unresolved so that we aren't exposing the partial state.
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.
Good point but is this really an issue or more a way to avoid people relying on unreliable information? But having an internal flag marking the graph as incomplete would definitely be a future-proof way of handling this. Created #2598 to track. There may be more opinions on this issue.