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

Nodent (and fast-async) break in Meteor #102

Open
trusktr opened this issue Oct 9, 2017 · 6 comments
Open

Nodent (and fast-async) break in Meteor #102

trusktr opened this issue Oct 9, 2017 · 6 comments

Comments

@trusktr
Copy link

trusktr commented Oct 9, 2017

I'm running code in Meteor, and I get

Uncaught SyntaxError: Unexpected token )

from the processIncludes function.

This is because Meteor appends a commented line number to all source code, even from node_modules, so for example

function foo() {
  blah()
}

becomes

function foo() { // 1
  blah()         // 2
}                // 3

When processIncludes concatenates the lines of the input, the comments cause the remainder of the function to be commented out, for example:

function foo() { // 1 blah()         // 2 }                // 3

See here for more details: meteor/meteor#9160

@trusktr
Copy link
Author

trusktr commented Oct 9, 2017

On another note, using new Function might not be a good idea, because with certain Content Security Policy headers present in a web app, the browser will block eval and similar mechanisms from being usable, which will cause processIncludes to fail also.

@trusktr
Copy link
Author

trusktr commented Oct 9, 2017

For now, I can get by with compiler: {promises: true, noRuntime: true}, useRuntimeModule: false for fast-async, but I thought that compiler: {promises: false, noRuntime: false}, useRuntimeModule: true would be better, but in the latter case I get the above issue.

There must be a better way to put things together for nodent-runtime without string source mangling at runtime?

@trusktr
Copy link
Author

trusktr commented Oct 9, 2017

What's the best fast-async config for IE 11? From my limited knowledge, it seems like compiler: {promises: false, noRuntime: false}, useRuntimeModule: true results in the smallest code with the runtime being required/imported by the files.

@matAtWork
Copy link
Collaborator

I don't have a planned fix for this right now. The processIncludes routine is not very clever, but works under all versions of node without issue. It could be replaced with a pre-build phase, but this would involve a much deeper dependency tree (for webpack or rollup or similar) which I don't want to introduce.

If you want to submit a PR that fixes processIncludes in this case, or makes it's use optional via a flag/option, I'm happy to review and potentially ship.

@matAtWork
Copy link
Collaborator

matAtWork commented Oct 16, 2017

Thinking about this over the weekend, you could try replacing the first regex at https://github.com/MatAtBread/nodent-runtime/blob/master/runtime.js#L35 with \(\/\*[^*]*\*\/|\/\/[^\n]*)\

TBH, I don't have the same setup as you, so I don't have a ready test-case to hand. However, if you pull (or fork) the nodent-runtime repo, npm i, make that change and then install into your project locally (e.g npm i ../nodent-runtime) you should be able to tell if it works. If it does, submit a PR and I'll test & ship the update to npm.

@trusktr
Copy link
Author

trusktr commented Jan 4, 2019

I'll be getting back to this soon!

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