[nicolo-ribaudo]

While investigating #16678, I spent some time checking what were the breaking changes in es-module-lexer@2.0.0 and how they affect Vite.

The only breaking change is that it switched from parsing import assertions (import ... assert {}) to parsing import attributes (import ... with {}): this does not affect Vite in any way, because Vite does not use es-module-lexer to read attributes.

This PR is effectively a no-op, but I'm opening it so that somebody else won't have to investigate again whether 2.0.0 contains breaking changes or not.

[bluwy]

We read the attribute location in one place:

vite/packages/vite/src/node/plugins/importAnalysis.ts

Lines 523 to 526 in 40a0847

	// strip import attributes as we can process them ourselves
	if (!isDynamicImport && attributeIndex > -1) {
	str().remove(end + 1, expEnd)
	}

IIUC es-module-lexer v1 handled both import assertions and attributes, but v2 only handles import attributes, so this would be somewhat breaking if someone were to still use import assertions. However given how long import assertions have been phased out, I'm ok with doing a small breaking change on the next minor.

[nicolo-ribaudo]

Oh I see, I was confused because es-module-lexer's v2 changelog mentions "support import attribute item parsing", I didn't realize that it still had the basic support in v1 already. It seems like we had zero tests for assert 😅

I agree that probably nobody uses assert anymore (since it works in no browser and no server-side runtime), but if you prefer we can also keep backwards compat by doing

          if(!isDynamicImport) {
            if (attributeIndex > -1) {
              str().remove(end + 1, expEnd)
            } else {
              const re = /\s*assert\b/y;
              re.lastIndex = expEnd;
              if (re.test(source)) {
                str().remove(end + 1, source.indexOf('}', expEnd) + 1)
            }
          }

Fyi I'm hoping that after this 2.0.0 update we can later update to 2.0.1 once it's released: right now there is a bug in es-module-lexer where it detects the attributes location in import(x, {}) differently from import("x", {}), causing Vite to accidentally drop them in the first case.

[bluwy]

I think I'm slightly inclined on not adding the compat since it'll run the regex on every normal imports otherwise, which could have some performance implication. But I'll see what others think of this, maybe it's fine for now and then we can drop it in the next minor.

I'll mark this for the next minor milestone for now so we track this.

[sapphi-red]

import assertions are removed from Chrome and Node a while ago so I guess it won't cause much problem. Let's try this in 8.1