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

fix: resolve various parser issues #7

Closed
wants to merge 1 commit into from
Closed

fix: resolve various parser issues #7

wants to merge 1 commit into from

Conversation

irudoy
Copy link

@irudoy irudoy commented Aug 15, 2020

* set `ecmaVersion` acorn option to `2020` (esxjs/esx#17)
* fix function attributes parsing (esxjs/esx#17)
* fix imported components registration (fix: #2)
* fix computed props
* add lockfile
@prograhammer
Copy link

prograhammer commented Aug 17, 2020

@irudoy I tried out this change (just directly added the code in node_modules to-esx) and still see it breaking on optional chaining (via babel-plugin-esx-ssr).

@irudoy
Copy link
Author

irudoy commented Aug 17, 2020

@prograhammer can you provide stack trace of error thrown with these changes?

@prograhammer
Copy link

@irudoy there's no new errors that appear with these changes, I'm just noticing that optional chaining still breaks. See here: esxjs/esx#17

@prograhammer
Copy link

prograhammer commented Aug 17, 2020

@irudoy BTW, I'm able to get through this issue by transpiling (via babel-loader) twice. I do one pass through babel on the optional chaining, etc, then another pass with babel-plugin-esx-ssr. However now I'm facing an issue with none of the components registering from esx:

ReferenceError: ESX: FooComponent not found in registered components","stack":"ReferenceError: ESX: 
FooComponent not found in registered components\n    at grow (webpack-
internal:///./node_modules/esx/lib/parse.js:116:13)\n    at parse (webpack-
internal:///./node_modules/esx/lib/parse.js:363:11)\n    at raw (webpack-internal:///./node_modules/esx/index.js:292:24)\n    
at render (webpack-internal:///./node_modules/esx/index.js:350:31)\n 

I get the above error with or without your changes.

Maybe related to this: #2

@prograhammer
Copy link

prograhammer commented Aug 17, 2020

@irudoy Now I'm transforming import/exports to commonjs (require). That get's me past the register errors, but now I'm getting ReferenceError: React is not defined.

@irudoy
Copy link
Author

irudoy commented Aug 18, 2020

@prograhammer first of all consider, that esx project isn't stable for now, and I'm also trying to get it to work with my codebase as well 😄 Personally, I'm not using babel-plugin-esx-ssr because it has issues, and with webpack it's more rational to use a loader to convert the whole module to esx.

Registration issues with imports should be resolved by this PR. Could you create an isolated repro? And for React is not defined too. Seems like it's related to your build tool setup.

@@ -46,10 +46,6 @@ profile-*
# VS Code
.vscode/

# lock files
yarn.lock
package-lock.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do want to ignore lock files, imo lock files are for applications, not modules

Copy link
Member

@davidmarkclements davidmarkclements left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm lock files (lock files do not belong in modules)

I think if this causes another part to break, even if that other part is experimental we need to at least isolate the cause and rationalize it (if not fix it) before we can merge this

@irudoy irudoy closed this Dec 9, 2020
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

Successfully merging this pull request may close these issues.

Missing esx.register of imported component
3 participants