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

Elsa sometimes fails to build #176

Closed
mmalenic opened this issue Jan 3, 2023 · 4 comments
Closed

Elsa sometimes fails to build #176

mmalenic opened this issue Jan 3, 2023 · 4 comments

Comments

@mmalenic
Copy link
Contributor

mmalenic commented Jan 3, 2023

Issue

When cloning the project, installing dependencies, and attempting to run the backed with npm run dev:mac or npm run watch:mac, compilation sometimes fails with an export/import syntax error:

/Users/marki/Documents/elsa-data-test/application/backend/node_modules/@umccr/elsa-types/error-types.ts:37
export const BASE_7807_ABOUT_BLANK = "about:blank";
^^^^^^

SyntaxError: Unexpected token 'export'
    at compileFunction (<anonymous>)
    at Object.compileFunction (node:vm:360:18)
    at wrapSafe (node:internal/modules/cjs/loader:1055:15)
    at Module._compile (node:internal/modules/cjs/loader:1090:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/marki/Documents/elsa-data-test/application/backend/node_modules/ts-node/src/index.ts:1608:43)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.Module._load (node:internal/modules/cjs/loader:839:12)
    at Module.require (node:internal/modules/cjs/loader:1028:19)
    at require (node:internal/modules/cjs/helpers:102:18)

This error appears when the backend compiles both elsa-types and elsa-constants. It looks like it is related to how node and ts-node treat CommonJS and ES6 modules.

Solution

This seems to be a very difficult error to track down and find the cause of, and after trying many different combinations of tsconfig.json and package.json options, I could not find an easy solution. There seems to be a mismatch between CommonJS and ES modules. Simple solutions like adding "esModuleInterop": true to tsconfig.json or setting "experimentalResolver": true in "ts-node" don't work.

I can't quite tell whether this is an error with node, ts-node, or typescript, however I think the problem is somewhere in the configuration.

The line prior to the error generates a warning which suggests a potential solution:

(node:32485) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

This would force node and ts-node to use ES modules, however it would require significant restructuring of all the imports in the backend. For example, relative imports would require a .js ending.

Workaround

A workaround for this issue is to build elsa-types and elsa-constants manually, and reinstall them as dependencies before running npm run dev:mac:

cd application/common/elsa-constants && tsc --build && cd -
cd application/common/elsa-types && tsc --build && cd -
cd application/backend && npm ci && npm run dev:mac

This allows ts-node to use the pre-compiled javascript.

I'm not sure why ts-node has an issue with this, but pre-compiling the typescript files fixes the problem. My understanding was that ts-node would use the same tsconfig.json as tsc.

@mmalenic mmalenic added the bug Something isn't working label Jan 3, 2023
@andrewpatto
Copy link
Member

Yeah but the advantage of not compiling the code (i.e. kind of the reason I like ts-node) is because you then don't need to be doing checks (i.e. Makefiles) to test that the generated .js files aren't older than the corresponding .ts files. If we pre-compile the common libraries - we then can end in situations where the rest of the project is compiling/running against JS that is out of date with the TS.

I realise ts-node failing is also equally bad! - and agree this is a complete mess - but not sure separately compiling is a big win..

@andrewpatto
Copy link
Member

Are you sure it isn't picking up the wrong tsc somehow.

All invokes need to have npx in front of them or it is liable to sometimes pick up globally instanlled instances..

@andrewpatto
Copy link
Member

Whilst I've encountered build problems from the React side (which we have no fixed with the new build tool) - I've never encountered this particular issue on running the backend side

(have encountered lots of modules/es6 errors when trying to do the Docker bundling - but that was a separate issue - in terms of literally running ts-node locally I haven't encountered this)

@mmalenic
Copy link
Contributor Author

mmalenic commented Jan 4, 2023

After removing and reinstalling node and npm, its working for me now. My npm version went from 9.2.0 -> 8.19.2.
When I upgrade npm from 8.19.2 -> 9.2.0 it shows the error again. I wonder what in npm 9.2.0 causes this to occur...

Thanks for helping me debug this!

@mmalenic mmalenic closed this as completed Jan 4, 2023
@mmalenic mmalenic removed the bug Something isn't working label Jan 4, 2023
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

No branches or pull requests

2 participants