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

Improve id resolution #2829

Merged
merged 59 commits into from May 3, 2019
Merged

Improve id resolution #2829

merged 59 commits into from May 3, 2019

Conversation

lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented May 1, 2019

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:
Resolves #2811

Description

This cleans up, refactors, fixes and extends the resolution of static and dynamic module ids.
It includes:

Fix existing this.resolveId plugin context function

Before when plugins returned false or the new object form in their resolveId hook, this function would also return false or an object. This is changed now in that this utility function will now always return the resolved id as a string or null if the id cannot be resolved. The documentation is adjusted as well.

Add new this.resolve plugin context function and mark this.resolveId and this.isExternal as deprecated in the documentation.

With the new object form, there are some situations that are very hard to manage via the existing utility functions. Most notably, this.isExternal did not take ids into account that were marked as "external" by plugins.

To that end, there now is a new this.resolve(source, importer): Promise({id: string, external: boolean}) utility function that resolves to an object and can be used to replace both this.resolveId and this.isExternal.

The old hooks have only been marked as deprecated in the documentation for now. I made notes here https://github.com/rollup/rollup/projects/2 and here https://github.com/rollup/rollup/projects/3 to deprecate and remove them in the next major versions.

Refine resolveDynamicImport hook

The hook has been refined in several important aspects and the documentation has been updated accordingly:

  • if the specifier is not a string and the hook returns a string, this string is used to replace the import argument (existing behaviour)
  • if the specifier is not a string and the hook returns false or all hooks return null, the import is retained as it is without a warning (existing behaviour)
  • if the specifier IS a string, the hook behaves exactly like this.resolveId:
    • returned strings will be interpreted as module references to be included in the graph (existing behaviour)
    • if all hooks return null, the specifier is passed to the normal resolveId hook (existing behaviour)
    • if the module is not resolved by either hook and the specifier was absolute, the same warning as for resolveId is shown (NEW)
    • if the module is not resolved by either hook and the specifier was relative, the same error as for resolveId is thrown (NEW)
  • Independent from whether the specifier is a string, the hook can always return an {id, external} object which will always be treated as a module reference (NEW). This allows resolving truly dynamic imports to modules in the graph.

points and introducing a manualChunkAlias for colouring to resolve this
confusing double use of chunkAlias
* Allow manual chunks to contain entry points without name or with the same name
* Throw if an emitted chunk is not found
* Throw if there is a conflict between manual chunk entries
* Allow nesting of manual chunks without requiring a specific order
- if the alias matches, the manual chunk becomes the entry chunk
- otherwise a facade is created
@lukastaegert lukastaegert changed the base branch from add-entry to master May 3, 2019 04:51
# Conflicts:
#	docs/05-plugins.md
#	src/Chunk.ts
#	src/ModuleLoader.ts
#	src/rollup/types.d.ts
#	src/utils/assetHooks.ts
#	src/utils/defaultPlugin.ts
#	src/utils/error.ts
#	src/utils/pluginDriver.ts
#	test/function/samples/emit-chunk/chunk-not-found/_config.js
@lukastaegert lukastaegert merged commit 856707c into master May 3, 2019
@lukastaegert lukastaegert deleted the improve-id-resolution branch May 3, 2019 05:15
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.

{ external: false } in resolveId passed incorrectly
1 participant