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

Module 101|ne:1023 not found. potentially erroring on | character in step parameters #1040

Open
jywarren opened this issue Apr 24, 2019 · 30 comments · May be fixed by #1634
Open

Module 101|ne:1023 not found. potentially erroring on | character in step parameters #1040

jywarren opened this issue Apr 24, 2019 · 30 comments · May be fixed by #1634

Comments

@jywarren
Copy link
Member

In working on this: publiclab/image-sequencer-app#6 (comment)

I got this error about not being able to parse parameters for the webgl-distort module:

Module 101|ne:1023 not found.

Full error here:

Module 101|ne:1023 not found.
TypeError: Cannot read property 'description' of undefined 
at insertStep (/srv/node_modules/image-sequencer/src/InsertStep.js:23:50) 
at InsertStep (/srv/node_modules/image-sequencer/src/InsertStep.js:61:3) at AddStep (/srv/node_modules/image-sequencer/src/AddStep.js:3:33) 
at Object.addSteps (/srv/node_modules/image-sequencer/src/ImageSequencer.js:75:29) 
at eachStep (/srv/node_modules/image-sequencer/src/Strings.js:132:17) 
at Array.forEach (<anonymous>) 
at Object.importString (/srv/node_modules/image-sequencer/src/Strings.js:131:21) 
at Object.sequencerInstance.loadImages (/srv/index.js:64:27) 
at /srv/node_modules/image-sequencer/src/ImageSequencer.js:179:23 
at /srv/node_modules/image-sequencer/src/ui/LoadImage.js:67:7
@Divy123
Copy link
Member

Divy123 commented Apr 25, 2019

@jywarren can you please provide some description here.
Thanks!!

@vibhorgupta-gh
Copy link

vibhorgupta-gh commented Apr 25, 2019

@Divy123 It's an extension to #1033 , i was working on that one. will update as soon as i find a fix

@jywarren
Copy link
Member Author

jywarren commented Apr 25, 2019 via email

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

Looking into this now!

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

@VibhorCodecianGupta Anything you would like to add here?

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

@jywarren I am seeing this error, do you think this can be linked?
Screen Shot 2019-05-08 at 9 48 47 PM

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

Looks like I found one of the issues @publiclab/is-reviewers
The name of fisheye-gl module is being used as "fisheye gl" which is why adding fisheye gl causes an error!

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented May 8, 2019 via email

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

Okay this is not specific to fisheye gl
Screen Shot 2019-05-08 at 10 05 33 PM
cc @jywarren

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

Yeah I am trying to isolate the error!

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

@harshkhandeparkar I think we might have to roll back a few commits here the problem appears to be deep rooted.

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

I'll try to figure out which commit added this code.

@harshkhandeparkar
Copy link
Member

I think @teisenhower had tried to rearrange the module names. That is maybe when the error started?

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented May 8, 2019

I think @teisenhower had tried to rearrange the module names in alphabetical order. That is maybe when the error started?

Sent from my OnePlus3 using FastHub

@harshkhandeparkar
Copy link
Member

This only happens in the demo though. CLI works. This is a BIG issue. Also urlHash works, so it is only the selector that is broken.

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

Okay, a lot of code has been pushed in since then I think. I'll open a new issue for this then.

@harshkhandeparkar
Copy link
Member

👍Ok

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

Harsh do you have any insight on the parsing error here?

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

I am trying to replicate it locally.

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented May 8, 2019 via email

@tech4GT
Copy link
Member

tech4GT commented May 8, 2019

I'm not really involved with soc so I'm not sure, sorry.

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented May 8, 2019 via email

@jywarren
Copy link
Member Author

jywarren commented May 8, 2019

Hi folks, this issue is quite long now so there are plenty of questions or topics that may need to be broken out into their own so we can follow up with them. I'll focus on the debugging of the parsing error with | characters.

message port closed

I don't think that's it, but it could be - do you think the pipe character is in some way being read as a Linux pipe? But the error is not showing breaking on the pipe, it's breaking on either a . or a :, it looks like.

Harsh - https://sequencer.publiclab.org/examples/#steps=contrast{} is working fine right now. Please open a new issue if you see an unrelated bug, thanks!

fisheye gl - that sounds bad, and would love a PR for this, and improved UI tests should catch something like this. But in the demo right now, it does seem to work for me: https://sequencer.publiclab.org/examples/#steps=contrast{},fisheye-gl{x:1|y:1}

You can see who touched files in the examples folder here: https://github.com/publiclab/image-sequencer/commits/main/examples

@jywarren
Copy link
Member Author

jywarren commented May 8, 2019

Noting that we published to gh-pages on April 19th (https://github.com/publiclab/image-sequencer/tree/gh-pages), but the commit by teisenhower you're mentioning was April 7th. So it seems unlikely or these errors would be appearing on the online demo right now.

@teisenhower
Copy link

Hey all

Sorry I'm just now seeing this.

Not sure if this will help or not but when I worked on that previous dropdown issue I didn't rearrange things to alphabetical order.

Please let me know if there is anything I can do to help with this.

Thanks!

@jywarren
Copy link
Member Author

jywarren commented May 8, 2019 via email

@harshkhandeparkar
Copy link
Member

Sorry @teisenhower.

@teisenhower
Copy link

@jywarren No problem!

@harshkhandeparkar No worries! Just wanted to make sure I could help if possible. Thanks!

@jywarren
Copy link
Member Author

Hi @harshkhandeparkar, I agree with refactoring UI code, but a) let's address in a separate issue, and b) expanded UI tests would be a great first step before potentially breaking UI functionality, what do you think?

@jywarren
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants