marko-js / marko

A declarative, HTML-based language that makes building web apps fun
https://markojs.com/
MIT License
13.25k stars 643 forks source link

[marko 5] TypeError: Cannot read property 'id' of undefined #1669

Closed redonkulus closed 2 years ago

redonkulus commented 3 years ago

Marko Version

5.1.18

Details

We use marko on our node.js (v12) Express application. In marko 4, our application would render marko templates properly, with no errors. After upgrading to marko 5 and making no other changes to the application, we now are getting the TypeError: Cannot read property 'id' of undefined error (stack trace below).

The issue occurs on this line of code. I logged the data and it is undefined. Template execution halts and the node process is restarted.

I know this error is vague without a smaller reproducible example. I'm looking for pointers on what would cause this code path execution so I can help debug better.

The same line of code in marko 4 does not get executed.

Expected Behavior

No errors are produced.

Actual Behavior

Stack trace above is produced only in marko 5. Same code execution in marko 4 does not produce this error.

Possible Fix

This is not ideal, but just changing the lines below resolves the issue completely and does not break the application in anyway.

From:

parentComponentDef.id + "-" + parentComponentDef.___nextKey(key),

To:

parentComponentDef?.id + "-" + parentComponentDef?.___nextKey(key),
Additional Info ### Your Environment Our app is built on Express via the recommended [marko integration guide](https://markojs.com/docs/express/). - Environment name and version (e.g. Chrome 39, node.js 5.4): node v12.16.2 - Operating System and version (desktop or mobile): mac desktop - Link to your project: Internal source project ### Steps to Reproduce I will try like to provide a reproducible example, but our application is pretty complex and uses marko's `await` tag exclusively to provide markup to the main marko page template. ### Stack Trace ```bash TypeError: Cannot read property 'id' of undefined at dynamicTag (/app/node_modules/marko/dist/runtime/helpers/dynamic-tag.js:111:32) at renderBody (/app/node_modules/@vzmi/kaizen-components/src/kaizen-module-rmp/index.marko:108:37) at renderContents (/app/node_modules/marko/dist/core-tags/core/await/renderer.js:222:9) at renderContents (/app/node_modules/marko/dist/core-tags/core/await/renderer.js:232:16) at renderBody (/app/node_modules/marko/dist/core-tags/core/await/renderer.js:183:5) at notifyCallbacks (/app/node_modules/marko/dist/core-tags/core/await/AsyncValue.js:37:7) at AsyncValue.w_ (/app/node_modules/marko/dist/core-tags/core/await/AsyncValue.js:115:7) at internal/process/task_queues.js:149:7 at AsyncResource.runInAsyncScope (async_hooks.js:197:9) at AsyncResource.runMicrotask (internal/process/task_queues.js:146:8) ```
redonkulus commented 2 years ago

No longer an issue, seemed to resolve itself when trying the upgrade recently.

snyamathi commented 2 years ago

I was able to reproduce this issue with the root cause being optimizations enabled for some compiled templates, but not others.

In my case, it was some templates statically compiled using markoc with a build process where NODE_ENV is set to production (enabling optimizations) and then using @marko/compiler/runtime to load other templates on the fly.

In production, this is fine, but in dev this would lead to some cases where we load the template, compiled without optimizations. It seems like there is some variable mangling with optimizations that makes the two not compatible, which makes sense.

tl;dr make sure you're consistent with optimizations - which if otherwise unset is a function of NODE_ENV

snyamathi commented 2 years ago

@DylanPiercey if you're interested, here's the repro repo https://github.com/snyamathi/markojs-marko-1669

I don't know that there's any reason why it would be valid to mix optimize true|false so likely no changes needed on your part, but I have seen other places where this is at least attempted to be caught, so it would be great if we could throw the same Runtime/NODE_ENV Mismatch error here, too.

https://github.com/marko-js/marko/blob/ab1d038070e1eb23323d0b790e68489ade280c3e/packages/marko/src/runtime/components/renderer.js#L72