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

Zipping projects and dependencies does not like certain tokens/features of more recent Node versions #614

Open
newhouse opened this issue Jan 30, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@newhouse
Copy link

Bug Description

I have updated to version 13 of both Zapier Core and CLI. That version states that it will run on Node 16, so:

  • I am now developing locally on Node 16
  • I changed my Babel preset env to target Node 16

As a result, now in my transpiled code I have a nullish coalescing assignment, which is a language feature that was added in Node 15 according to MDN.

Yet, when I run zapier build, it will complain about an Unexpected token

⠼ Zipping project and dependenciesnode:events:505
      throw er; // Unhandled 'error' event
      ^

Error: Parsing file /private/var/folders/8s/c2_5kv4j23bd5gbk2z47n4xr0000gn/T/zapier-9978a26b/dist/resources/Field.js: Unexpected token (160:33)
    at Deps.parseDeps (/Users/xxx/projects/xxx/zapier/node_modules/module-deps/index.js:519:15)
    at getDeps (/Users/xxx/projects/xxx/zapier/node_modules/module-deps/index.js:447:44)
    at /Users/xxx/projects/xxx/zapier/node_modules/module-deps/index.js:430:38
    at ConcatStream.<anonymous> (/Users/xxx/projects/xxx/zapier/node_modules/concat-stream/index.js:37:43)
    at ConcatStream.emit (node:events:539:35)
    at ConcatStream.emit (node:domain:475:12)
    at finishMaybe (/Users/xxx/projects/xxx/zapier/node_modules/readable-stream/lib/_stream_writable.js:630:14)
    at endWritable (/Users/xxx/projects/xxx/zapier/node_modules/readable-stream/lib/_stream_writable.js:638:3)
    at ConcatStream.Writable.end (/Users/xxx/projects/xxx/zapier/node_modules/readable-stream/lib/_stream_writable.js:594:41)
    at DuplexWrapper.onend (/Users/xxx/projects/xxx/zapier/node_modules/readable-stream/lib/_stream_readable.js:577:10)
    at Object.onceWrapper (node:events:641:28)
    at DuplexWrapper.emit (node:events:539:35)
    at DuplexWrapper.emit (node:domain:475:12)
    at endReadableNT (/Users/xxx/projects/xxx/zapier/node_modules/readable-stream/lib/_stream_readable.js:1010:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)
Emitted 'error' event on Readable instance at:
    at Labeled.<anonymous> (/Users/xxx/projects/xxx/zapier/node_modules/read-only-stream/index.js:28:44)
    at Labeled.emit (node:events:527:28)
    at Labeled.emit (node:domain:475:12)
    at Labeled.<anonymous> (/Users/xxx/projects/xxx/zapier/node_modules/stream-splicer/index.js:130:18)
    at Labeled.emit (node:events:527:28)
    at Labeled.emit (node:domain:475:12)
    at Deps.<anonymous> (/Users/xxx/projects/xxx/zapier/node_modules/stream-splicer/index.js:130:18)
    at Deps.emit (node:events:527:28)
    at Deps.emit (node:domain:475:12)
    at /Users/xxx/projects/xxx/zapier/node_modules/module-deps/index.js:414:22
    at /Users/xxx/projects/xxx/zapier/node_modules/module-deps/index.js:431:35
    at ConcatStream.<anonymous> (/Users/xxx/projects/xxx/zapier/node_modules/concat-stream/index.js:37:43)
    [... lines matching original stack trace ...]

It seems to me like module-deps needs to be able to understand and allow Node 16, etc, syntax? This syntax is OK in Node 16 for sure.

Reproduction Steps

  1. Be on Zapier 13
  2. Be on Node 16
  3. Put a nullish coalescing assignment into your code somewhere
  4. Run zapier build.

Version Info

  • Version info:
* CLI version: 13.0.0
* Node.js version: v16.15.1
* OS info: darwin-x64
* `zapier-platform-core` dependency: 13.0.0
✨  Done in 0.21s.
  • Operating System: MacOS 12.6
  • App id:
@rnegron
Copy link
Member

rnegron commented Jan 30, 2023

@newhouse! Thank you for taking the time to report this issue and for including such a detailed description! I'll be taking a look at this issue and will report back here once we have a fix in place.

@rnegron rnegron self-assigned this Jan 30, 2023
@rnegron rnegron added the bug Something isn't working label Jan 30, 2023
@rnegron
Copy link
Member

rnegron commented Jan 30, 2023

This appears to be an issue with our browserify dependency, see related issues here and here. Unfortunately, browserify hasn't seen a release since late 2020. Looking into potential solutions...

@newhouse
Copy link
Author

@rnegron Thanks!!!

After poking around a bit I don't have high hopes for this getting fixed in browserify, and as time goes on their problem with modern syntax will become your Users' problems with modern syntax.

You may want to consider another bundler? I don't have any to suggest and I know that's not a simple change, but it may be the only way.

Perhaps another option might be to have a temporary babel transpilation step in your bundling and dependency stuff that targets something like Node 14 or whatever is the most accommodating for the shortcomings of browserify. But I would think you would not want to actually upload the transpiled code or you'd have all sorts of drawbacks like: source code mapping in errors, lack of native, non-transpiled efficiency and optimizations, things that cannot be transpiled (top level await?), etc.

Just thoughts. Thanks for working on this!

@rnegron
Copy link
Member

rnegron commented Feb 15, 2023

Hello @newhouse, wanted to give you a quick update on where we are on this issue. After determining that this is mainly related to the logical assignment operators introduced in Node.js v15, and seeing as there is potential for this to get fixed upstream, we have de-prioritized this issue for the moment. As I know you're aware, a temporary workaround around this is to set up a transpilation pipeline using babel or similar.

I left a message in the open PR (browserify/detective#85 (comment)) that would unblock this, and will keep an eye out for opportunities to prioritize this fix in the Zapier CLI.

@newhouse
Copy link
Author

Thanks for the update @rnegron!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants