flow icon indicating copy to clipboard operation
flow copied to clipboard

Frontend broke with VAADIN 23.2.0 due to Vite being used and not allowing `require` anymore

Open DManstrator opened this issue 2 years ago • 16 comments

Description of the bug

After updating from VAADIN 23.1.7 to VAADIN 23.2.0, the (main page of the) application can't be displayed anymore. In the Chrome console, the error is simple:

Uncaught (in promise) ReferenceError: require is not defined at generated-flow-imports.9cd14ffe.js:formatted:58387:10

As far as I know, VAADIN 23.2.0 started to use Vite instead of Webpack where Vite simply doesn't allow require to be used due to ES5 compatibility(?).

We had several usages of require in our frontend where I was able to eliminate most of them, except for 2 usages.

  • cytoscape-expand-collapse (has no module support yet)
  • cytoscape-node-html-label (has invalid module support)

I researched a lot for possible solutions finding - amongst others - the following issue: https://github.com/vitejs/vite/issues/3409 Sadly, none of the mentioned solutions (using the vite-plugin-commonjs plugin and using transformMixedEsModules: true) worked out for me.

I also tried setting "module": "commonjs" in my tsconfig.json but that didn't work out either due to exporting my own classes / components.

I know this is not really a bug but how are we supposed to migrate our projects in such a case? In my case the TS file throwing the error is nearly 3 years old and the last change was over a year ago.

Minimal reproducible example

See this example project: project-with-require.zip (with Git history)

On commit cb4143d9cbc484cb2c2aaed59977737f174409a8, the project works as expected when using VAADIN 23.1.7. After updating to VAADIN 23.2.0 (ec6e6b64e46b622eb8641634ec5fe40455524558, state of the ZIP), the same error message from above will be displayed in the Browser console.

Expected behavior

I expect the application to show the main page.

Actual behavior

Even the main page is not showing up anymore, the site looks like it's loading forever. An error can be found in the Browser console.

image

Versions:

- Vaadin / Flow version: VAADIN 23.2.0
- Java version: 17.0.2 2022-01-18 LTS
- OS version: Windows 10 21H1
- Browser version (if applicable): Not needed.
- Application Server (if applicable): Not needed.
- IDE (if applicable): Not needed.
- Development or production mode: Both.
- Project from start.vaadin.com or activated in application: See minimal reproducible example above.

DManstrator avatar Sep 05 '22 18:09 DManstrator

For the meantime you could still use webpack which is behind a "feature / deprecation" flag to allow the flow team to find a proper solution and still be on the latest version

knoobie avatar Sep 05 '22 18:09 knoobie

You can use import syntax even if the modules are written as common js.

Your project seems to work if you replace the require statements with

// @ts-ignore
import * as nodeHtmlLabel from 'cytoscape-node-html-label';
// @ts-ignore
import * as expandCollapse from 'cytoscape-expand-collapse';

and change the CustomComponent to use the TS file

@JsModule("./src/folder1/component.ts")

Artur- avatar Sep 08 '22 11:09 Artur-

@Artur- This may work for his example, but if you depend on other modules that use require themselves, you cannot rewrite the 'require' to 'import' statements.

Example of one of many modules in my project that call 'require': https://www.npmjs.com/package/mime-types

This change/bug basically prohibits the use of many, maybe most third party node modules.

mrgreywater avatar Sep 08 '22 11:09 mrgreywater

I don't see any problem with that either

npm i mime-types

and

// @ts-ignore
import * as mime from 'mime-types';

and

  render() {
    return html`<p>Hello, ${mime.lookup('json')}!</p>`;
  }

in your project and the page prints "Hello, application/json!" once you have applied https://dev.to/0xbf/vite-module-path-has-been-externalized-for-browser-compatibility-2bo6 as the mime-types package is depending on nodejs only API

Artur- avatar Sep 08 '22 11:09 Artur-

Seems somewhat alright, even though you also need to manually install the npm packages now, they are not automatically picked up using NpmPackage() in java anymore. Also I have no idea how you would rewrite a callable require like here https://www.npmjs.com/package/fetch-retry to an import statement now...

mrgreywater avatar Sep 08 '22 11:09 mrgreywater

you also need to manually install the npm packages now, they are not automatically picked up using NpmPackage() in java anymore.

Why would you claim that? It is not true

Artur- avatar Sep 08 '22 11:09 Artur-

I have no idea how you would rewrite a callable require like here https://www.npmjs.com/package/fetch-retry to an import statement now...

Do you mean this

var originalFetch = require('isomorphic-fetch');
var fetch = require('fetch-retry')(originalFetch);

which is equivalent to

var originalFetch = require('isomorphic-fetch');
var f = require('fetch-retry');
var fetch = f(originalFetch);

which is equivalent to

import * as originalFetch from 'isomorphic-fetch';
import * as f from 'fetch-retry';
var fetch = f(originalFetch);

?

Artur- avatar Sep 08 '22 11:09 Artur-

Well it's true for me, without vite it works fine with just the NpmPackage() definition, with Vite, it doesn't. When running it complains the package doesn't exist, and prompts to install it with npm -i --save-dev . After it's manually installed it works. edit: the new typescript checker throws an error that he can't find the module, it works fine when suppressing the error, so the module exists.

Also:

import * as originalFetch from 'isomorphic-fetch';
import * as f from 'fetch-retry';
var fetch = f(originalFetch);

doesn't work, typescript complains typeof f is not callable. I tried before. With require it just worked.

mrgreywater avatar Sep 08 '22 12:09 mrgreywater

Weill it works when added to the example project referenced here. You might need to add // @ts-ignore if typescript isn't able to determine the types correctly

Artur- avatar Sep 08 '22 12:09 Artur-

If you use import f from 'fetch-retry'; instead it seems TypeScript gets the type correctly

Artur- avatar Sep 08 '22 12:09 Artur-

So @DManstrator , is there something to fix here still?

Artur- avatar Sep 08 '22 16:09 Artur-

Hello Artur,

thanks for all the quick feedback. I tried what you recommended in the real project but now the build even fails during the build-frontend goal.

[ERROR] [36mvite v3.0.9 building for production...
[ERROR] transforming...
[ERROR] node_modules/@types/node/globals.d.ts(172,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'module' must be of type 'any', but here has type 'NodeModule'.
[ERROR] node_modules/cytoscape-node-html-label/dist/cytoscape-node-html-label.d.ts(5,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'cytoscape' must be of type 'typeof cytoscape', but here has type 'any'.
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal com.vaadin:vaadin-maven-plugin:23.2.0:build-frontend (default) on project projectName: Execution default of goal com.vaadin:vaadin-maven-plugin:23.2.0:build-frontend failed: Vite process exited with non-zero exit code.
Stderr: 'vite v3.0.9 building for production...
transforming...
node_modules/@types/node/globals.d.ts(172,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'module' must be of type 'any', but here has type 'NodeModule'.
node_modules/cytoscape-node-html-label/dist/cytoscape-node-html-label.d.ts(5,13): error TS2403: Subsequent variable declarations must have the same type.  Variable 'cytoscape' must be of type 'typeof cytoscape', but here has type 'any'.
'
[...]

I also tried deleting the node_modules folder and rebuilding it but it still fails.

In addition to, I get an additional error shorty before:

[INFO] Frontend dependencies resolved successfully.
[INFO] Copying frontend resources from jar files ...
[INFO] Visited 263 resources. Took 213 ms.
[ERROR] Invalid import './Graph'
import { AbstractComponent, MessageKey } from '../abstract-component'
//a few lines of the file with that import
                brea' in file 'C:\Users\[...]\frontend\src\folder1\component.ts'
[DEBUG] Failed to resolve path.
java.nio.file.InvalidPathException: Illegal char <
> at index 8: ./Graph'
import { AbstractComponent, MessageKey } from '../abstract-component'
[...]
[INFO] Reflections took 979 ms to scan 264 urls, producing 16173 keys and 87713 values
[INFO] Running Vite ...

It states that a <\n>(?) was found at index 8 and I have no idea how this is the case.

The beginning of the component.ts which imports the Graph.ts:

// path: frontend/src
import { Graph } from './Graph'
import { AbstractComponent, MessageKey } from '../abstract-component'
// [...]

Graph.ts is the file which contained the require statements which I changed to what you suggested.

Do you have any ideas for me what I can try? Do you need some of the TS files or more files in general?

DManstrator avatar Sep 09 '22 08:09 DManstrator

If you have not yet, try to delete and regenerate tsconfig.json first. Some of the errors sound like irrelevant typing errors in files inside node_modules

java.nio.file.InvalidPathException: Illegal char < at index 8: ./Graph'

On thing that comes to mind is #14472 which could cause problems if you have some old style Lit imports in the same file. That will be fixed in 23.2.1

Artur- avatar Sep 09 '22 08:09 Artur-

I deleted the tsconfig.json and the node_modules and tried it again, the project now can be built again.

Due to testing I found out that the needed change was adding "skipLibCheck": true to the tsconfig.json.

However, after opening the component using Cytoscape, I got a new error from one of the dependencies I used require on before. For now I commented that out to see if it would generally work (those dependency are just extensions). Sadly, cytoscape itself isn't useable anymore.

In the Graph.ts (which is getting imported in component.ts), I import Cytoscape with import * as cytoscape from 'cytoscape' (I already do that for a year now). I initialize Cytoscape as a field (outside of a constructor) with graph: cytoscape.Core = cytoscape(). This gets eventually translated to this.graph = sp() and the error I get in the console is TypeError: sp is not a function.

I investigated this for hours, here are my results:

  • To resolve this issue for Webpack, only replacing require with import as you have shown in your comment above is required. When using Vite, this is not enough. The project at least builds again when adding the "skipLibCheck": true setting. There is still at least the runtime error I mentioned above. Due to that I assume it's still related to Vite.

  • This "invalid import" error actually comes from referencing the TS file instead of the JS file in the JsModule annotation in the CustomComponent. When referencing the JS file, there is no "invalid import" error, both for Webpack and Vite.

On thing that comes to mind is https://github.com/vaadin/flow/pull/14472 which could cause problems if you have some old style Lit imports in the same file. That will be fixed in 23.2.1

My component is extending LitElement by importing from 'lit-element' so this seems to be the issue, yes.

Other question: Is there any advantage of referencing the TS file instead of the JS file? We're using that for 3 years now and never had issues before.

DManstrator avatar Sep 09 '22 15:09 DManstrator

I am not sure why you have configured "outDir": "frontend/target", in your tsconfig.json and why you would refer to the JS files there instead of the TS files? To me it looks like you are polluting the frontend folder with unnecessary content when you could just refer to the source files. If you just use the JS files from Flow, then it sounds quite clear that you would not see TS related warnings as you are not using the TS files. Without digging deeper It is not even clear to me why something in the toolchain compiles the TS files at all :)

Could you try with 23.2.1 where the Lit import problem is fixed along with some other bugs and if you still have a problem, create a new project that shows the issue?

Artur- avatar Sep 16 '22 07:09 Artur-

Hello Artur,

sorry for the late reply. I tested it again with VAADIN 23.2.1 but it doesn't fully work yet (see last point).

I am not sure why you have configured "outDir": "frontend/target", in your tsconfig.json

Normally (or at least what happend back then) the created .js and .js.map files - which are created by tsc - are added to the src folder. And to keep that clean we moved the generated content into a target folder.

and why you would refer to the JS files there instead of the TS files?

I honestly can't explain it since that was used before I joined the project. When I would have to guess, we didn't know any better.

Without digging deeper It is not even clear to me why something in the toolchain compiles the TS files at all :)

We use the postinstall script to perform a tsc which compiles the TS files to JS files so we can access them in the @JsModules just in time. That's also why I opened #13537 back then. But I see that this might be completely unneeded. I will check / change this as soon everything works again.

Could you try with 23.2.1 where the Lit import problem is fixed along with some other bugs and if you still have a problem, create a new project that shows the issue?

Sure, here: project-with-require_v2.zip

I used my prior example project and applied the requested changes over multiple commits. The current version builds and the application also starts, but when I navigate to the "Test" page (http://localhost:8080/test), I still get the TypeError: sp is not a function. error in the console as described in my comment above.

I also noticed something interesting which might be related to it. When the Lit Component itself imports cytoscape, I get an additional error while building:

[ERROR] Invalid import 'cytoscape'

import * as nodeHtmlLabel from 'cytoscape-node-html-label' in file 'C:\Users\[...]\project-with-require\frontend\.\src\folder1\component.ts'

However, the build won't fail due to that. You can see that state with commit 1cfe8324c52d21b4063c7e417918c68edf662595.

DManstrator avatar Sep 21 '22 12:09 DManstrator

Hello all,

I can report the same error. I can add the following info:

The error does not occur on my local PC (Win10 / 22H2 ). The error occurs on the BuildSystem (Hosted Linux) under Jenkins, when the BuildJob was triggered by Jenkins. After restarting the server, the build also works with Jenkins. A local build on the build server always works.

Very confused

mschulzeondts avatar Oct 23 '22 08:10 mschulzeondts

Is this still a problem? I cannot reproduce it

Artur- avatar Apr 04 '23 08:04 Artur-

I have worked around it using imports as you suggested, but if I try to use 'require' even in Vaadin 24.0.3 it still throws "ReferenceError: require is not defined". So it's resolved for me, as far as I have a workaround, but require is still not working.

You could reproduce it by adding @NpmPackage(value = "mime-types", version = "2.1.35") and then in javascript, execute var mime = require('mime-types'). It will fail, while

// @ts-ignore
import mime from 'mime-types';

will work.

mrgreywater avatar Apr 04 '23 10:04 mrgreywater

It is known that you cannot use require in project files as Vite is built around es-modules (standard) and not commonjs modules (not a standard), so it does not allow usage of require in the project files. But there shouldn't be anything you need require for as you can use the standard import instead. That part we cannot fix or change

Artur- avatar Apr 04 '23 10:04 Artur-

Then it seems this issue cannot actually get fixed by design. While I'm not OP I guess this issue is resolved. I'd love some better documentation on the topic though.

mrgreywater avatar Apr 04 '23 10:04 mrgreywater

Is this still a problem?

Yes, it's still a problem for me since not all (of my used) NPM dependencies support using "import" over "require".

I cannot reproduce it

So the provided example in my previous comment is not reproducible for you? In my case, I get an error in the Chrome Console and the used dependency (in this case: Cytoscape) does not work.

DManstrator avatar Apr 06 '23 15:04 DManstrator

Sure, here: project-with-require_v2.zip

I took this project, checked out head^, upgraded to Vaadin 23.3.8, reverted to the default tsconfig.json and the project starts without errors. Component updated. is logged to the browser console when opening the test view

Artur- avatar Apr 06 '23 16:04 Artur-

Interesting. Could you tell me what exactly your "default tsconfig.json" looks like now? And might the VAADIN update be the reason it works again? I'm still stuck on VAADIN 23.2 for multiple reasons and I'm currently trying to resolve that.

I will then try it next week again, if it is also resolved for me with latest VAADIN 23.3 and your tsconfig.json.

DManstrator avatar Apr 06 '23 20:04 DManstrator

This should allow you to use require in ESM.

import { createRequire } from 'module';
const require = createRequire(import.meta.url);

cromoteca avatar Apr 06 '23 21:04 cromoteca

Could you tell me what exactly your "default tsconfig.json" looks like now?

Delete tsconfig.json and the default will be created

Artur- avatar Apr 07 '23 04:04 Artur-

@Artur- I tested it right now. Funny enough, when checking out the latest commit and starting it without any other adjustments, I also got no error in the console but Component updated. appeared.

I had to checkout the previous commit (51515fc017477660d1999606425fbe5c92212159) where the application didn't load, build it in production mode, checkout the latest commit (b158ac08da3ab70fe68781c753bcd4ea5642ed2d) and build again in production mode. Only then the error appeared in the log.

However, I followed your steps (update to VAADIN 23.3.8 and regenerate the tsconfig.json) but now, an exception occurs when opening the application in the browser.

2023-04-11 11:31:48.750 ERROR 10556 --- [nio-8080-exec-4] c.v.flow.server.DefaultErrorHandler      : 

java.lang.IllegalArgumentException: The number of received values (6) is not equal to the number of arguments (5) in the method 'connectClient' declared in 'com.vaadin.flow.component.internal.JavaScriptBootstrapUI'
	at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.decodeArgs(PublishedServerEventHandlerRpcHandler.java:261) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.invokeMethod(PublishedServerEventHandlerRpcHandler.java:222) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.invokeMethod(PublishedServerEventHandlerRpcHandler.java:199) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.invokeMethod(PublishedServerEventHandlerRpcHandler.java:149) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.handleNode(PublishedServerEventHandlerRpcHandler.java:132) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.rpc.AbstractRpcInvocationHandler.handle(AbstractRpcInvocationHandler.java:75) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocationData(ServerRpcHandler.java:438) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.ServerRpcHandler.lambda$handleInvocations$1(ServerRpcHandler.java:419) ~[flow-server-23.2.1.jar:23.2.1]
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) ~[na:na]
	at com.vaadin.flow.server.communication.ServerRpcHandler.handleInvocations(ServerRpcHandler.java:419) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:320) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:115) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:40) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1564) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:364) ~[flow-server-23.2.1.jar:23.2.1]
	at com.vaadin.flow.spring.SpringServlet.service(SpringServlet.java:106) ~[vaadin-spring-23.2.1.jar:na]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:764) ~[tomcat-embed-core-9.0.65.jar:4.0.FR]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:227) ~[tomcat-embed-core-9.0.65.jar:9.0.65]
        [...]

Here the latest version of my example: project-with-require_v3.zip

DManstrator avatar Apr 11 '23 09:04 DManstrator

That exception is just not related and just a cache within your browser.

knoobie avatar Apr 11 '23 09:04 knoobie

@knoobie Thanks for the very fast answer. I restarted my PC and now the application loads again.

However, I still get an error in the console as before.

image

DManstrator avatar Apr 11 '23 10:04 DManstrator

@cromoteca Never knew something like this existed, cool.

However when I try to use it, the build fails.

[ERROR] Error: 'createRequire' is not exported by __vite-browser-external, imported by frontend/src/folder1/Graph.ts

Did you encounter such an error by any chance?

DManstrator avatar Apr 11 '23 10:04 DManstrator