[v3.0] Basic support for import assertions#4646
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#rollup-3-import-assertionsor load it into the REPL: |
|
@lukastaegert how does this work for inputs that have assertions when there is no special config? Say: import './foo.css' assert { type: 'css' };That's preserved in the output, right? |
|
No it is not. Otherwise it would get really complicated. This solution just focuses on consistent output. And I am not aware that CSS is already standardized? |
|
Hmm... I think this is the biggest use case for import assertions right now. A library may publish code that uses assertions and non-JS module types and by default they should continue to work after bundling as they did before. Yes, CSS modules are standardized (In the HTML spec here) and shipping in Chrome and Edge since v93. Firefox and Webkit were involved in the development of import assertions as a result of working out CSS modules, and are both currently implementing the prerequisites for CSS modules (constructible stylesheets and adopedStyleSheets). |
Fair enough, then we should also add them. Unfortunately I could not find an overview which assertions are supported where. Wasn't there also discussion about an |
|
The problem with keeping assertions is right now that it would blow up the feature with lots of edge cases to be considered. Assertions need to be tracked, stored on external module instances, we need to handle inconsistent assertions. At the same time it can be useful to add assertions even where there were no assertions before. Primary use case would be required JSON that is turned into an import by the commonjs plugin. Though one could argue that that could be the responsibility of the commonjs plugin. The beauty of the current version is that it is easy to turn off. |
|
This already looks really great. Thanks a lot for the time and effort you've put into this, and the quick response 🙂 However, as mentioned by Justin, my main usecase is this:
In case it helps, let me clarify my usecase again: Currently, I can use css import assertions in my code and it will work in browsers that support them, or via something like es-module-shims which also supports them. However, when I try to bundle my app with Rollup, it'll fail on the import assertion. So the issue is that I have valid browser code, which my bundler crashes on. Ideally what I'd like is for rollup to leave the following import intact: import './foo.css' assert { type: 'css' };OUTPUT CODE: import './foo.css' assert { type: 'css' }; |
This issue is already fixed by this PR. I will see if we can change the default to keep assertions instead in the next days, but as I said, quite a bit of work. |
b3b80e1 to
cc8d5f3
Compare
Codecov Report
@@ Coverage Diff @@
## release-3.0.0 #4646 +/- ##
==============================================
Coverage 99.05% 99.06%
==============================================
Files 213 214 +1
Lines 7476 7554 +78
Branches 2069 2096 +27
==============================================
+ Hits 7405 7483 +78
Misses 23 23
Partials 48 48
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
f65d37b to
1fbe2a6
Compare
|
I completely reworked the feature now. See the updated PR description. Basically this will now only take assertions that are already present in the input and warn about conflicting assertions. Plugins can however easily add or remove assertions at many points. There is only a simple new option that allows you to turn off assertions altogether, but for fine-grained control you need the plugin interface. Unless there are heavy concerns, I would plan to release this feature tomorrow together with Rollup 3. Incidentally, this makes the feature non-breaking 😄 |
|
Judging from the updated description this indeed matches with how I'd expect this to work 🙂 Looking forward to trying this out. Thanks a lot for the discussion, and for the effort towards making this happen! |
170129a to
685d10c
Compare
|
Wow, this was quick work @lukastaegert! Agreed with @thepassle that this looks like what I need out of assertion support. This should allow for one build with no plugins for browsers with native CSS modules support and another build with a plugin to polyfill it, for instance. Thanks for adjusting the PR!! |
|
Thanks a lot for your efforts here @lukastaegert this really helps a lot to push standards-based development further! |
* Add acorn support for import assertions and extend AST * Ignore pre-existing assertions on input files * Naive support for JSON assertions in output * Inject arbitrary import assertions * Allows to disable import assertions altogether via `false` * Support shorthand syntax for type assertions * Keep assertions on fully dynamic imports * Add documentation * Add assertions to types * Keep original assertions * Make option a boolean * Some extractions * Allow plugins to add and change assertions * Allow to pass assertions in this.resolve * Warn for inconsistent import assertions * Add new documentation * Improve coverage
|
This PR has been released as part of rollup@3.0.0-8. Note that this is a pre-release, so to test it, you need to install Rollup via |
* Add acorn support for import assertions and extend AST * Ignore pre-existing assertions on input files * Naive support for JSON assertions in output * Inject arbitrary import assertions * Allows to disable import assertions altogether via `false` * Support shorthand syntax for type assertions * Keep assertions on fully dynamic imports * Add documentation * Add assertions to types * Keep original assertions * Make option a boolean * Some extractions * Allow plugins to add and change assertions * Allow to pass assertions in this.resolve * Warn for inconsistent import assertions * Add new documentation * Improve coverage
|
This PR has been released as part of rollup@3.0.0. You can test it via |
Rollup v3 has introduced some support of import assertions in rollup/rollup#4646. In particular, it uses an `{ assertions: { type: <xxx> } }` object to track the assertions linked to a module. However, this does not seem to be set in preload, whereas it _is_ set afterwards. This results in a warning. In order to silence the warning, we need to explicitly tell rollup about the assertions type, so that this piece of information is available even at preload time. Note: this also makes the test 'handles garbage' pass. Without this rollup issues 2 warnings instead of 1 expected (resulting in test failure).

BREAKING CHANGE: By default, this will add import assertions to all external
.jsonfile imports in the output.This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
assert { type: 'json' }results in "Error: Unexpected token" #4530Description
This PR adds basic support for import assertions to Rollup.
this.getModuleInfo.output.externalImportAssertionsoption that allows to disabled assertions in the output.resolveIdandresolveDynamicImport. If plugins do not return a value forassertionsin theirresolveIdhook, the assertions from the actual import are used, otherwise they can override assertions at this point. For symmetry with other module options, they can also be changed in theloadandtransformhooks. Inthis.resolve, you can now also pass assertions that will be forwarded toresolveId.output.externalImportAssertions
Type:
booleanCLI:
--externalImportAssertions/--no-externalImportAssertionsDefault:
trueWhether to add import assertions to external imports in the output if the output format is
es. By default, assertions are taken from the input files, but plugins can add or remove assertions later. E.g.import "foo" assert {type: "json"}will cause the same import to appear in the output unless the option is set tofalse. Note that all imports of a module need to have consistent assertions, otherwise a warning is emitted.resolveIdType:
(source: string, importer: string | undefined, options: {isEntry: boolean, assertions: {[key: string]: string}, custom?: {[plugin: string]: any}}) => string | false | null | {id: string, external?: boolean | "relative" | "absolute", assertions?: {[key: string]: string} | null, meta?: {[plugin: string]: any} | null, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null}…
assertionstells you which import assertions were present in the import. I.e.import "foo" assert {type: "json"}will pass alongassertions: {type: "json"}.…
If you return a value for
assertionsfor an external module, this will determine how imports of this module will be rendered when generating"es"output. E.g.{id: "foo", external: true, assertions: {type: "json"}}will cause imports of this module appear asimport "foo" assert {type: "json"}. If you do not pass a value, the value of theassertionsinput parameter will be used. Pass an empty object to remove any assertions. Whileassertionsdo not influence rendering for bundled modules, they still need to be consistent across all imports of a module, otherwise a warning is emitted. Theloadandtransformhooks can override this.resolveDynamicImportType:
(specifier: string | ESTree.Node, importer: string, {assertions: {[key: string]: string}}) => string | false | null | {id: string, external?: boolean | "relative" | "absolute", assertions?: {[key: string]: string} | null, meta?: {[plugin: string]: any} | null, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null}…
assertionstells you which import assertions were present in the import. I.e.import("foo", {assert: {type: "json"}})will pass alongassertions: {type: "json"}.loadType:
(id: string) => string | null | {code: string, map?: string | SourceMap, ast? : ESTree.Program, assertions?: {[key: string]: string} | null, meta?: {[plugin: string]: any} | null, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null}…
assertionscontain the import assertions that were used when this module was imported. At the moment, they do not influence rendering for bundled modules but rather serve documentation purposes. Ifnullis returned or the flag is omitted, thenassertionswill be determined by the firstresolveIdhook that resolved this module, or the assertions present in the first import of this module. Thetransformhook can override this.shouldTransformCachedModuleType:
({id: string, code: string, ast: ESTree.Program, resolvedSources: {[source: string]: ResolvedId}, assertions: {[key: string]: string}, meta: {[plugin: string]: any}, moduleSideEffects: boolean | "no-treeshake", syntheticNamedExports: boolean | string}) => booleantransformType:
(code: string, id: string) => string | null | {code?: string, map?: string | SourceMap, ast? : ESTree.Program, assertions?: {[key: string]: string} | null, meta?: {[plugin: string]: any} | null, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null}…
assertionscontain the import assertions that were used when this module was imported. At the moment, they do not influence rendering for bundled modules but rather serve documentation purposes. Ifnullis returned or the flag is omitted, thenassertionswill be determined by theloadhook that loaded this module, the firstresolveIdhook that resolved this module, or the assertions present in the first import of this module.this.getModuleInfoType:
(moduleId: string) => (ModuleInfo | null)Returns additional information about the module in question in the form
this.resolveType:
(source: string, importer?: string, options?: {skipSelf?: boolean, isEntry?: boolean, assertions?: {[key: string]: string}, custom?: {[plugin: string]: any}}) => Promise<{id: string, external: boolean | "absolute", assertions: {[key: string]: string}, meta: {[plugin: string]: any} | null, moduleSideEffects: boolean | "no-treeshake", syntheticNamedExports: boolean | string>…
If you pass an object for
assertions, it will simulate resolving an import with an assertion, e.g.assertions: {type: "json"}simulates resolvingimport "foo" assert {type: "json"}. This will be passed to anyresolveIdhooks handling this call and may ultimately become part of the returned object.this.loadType:
({id: string, resolveDependencies?: boolean, assertions?: {[key: string]: string} | null, meta?: {[plugin: string]: any} | null, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null}) => Promise<ModuleInfo>