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 compilation on fresh installs #155

Merged
merged 1 commit into from
Jul 19, 2023
Merged

Conversation

vadimavdeev
Copy link
Contributor

Local builds no longer work because esbuild has been upgraded to a backward-incompatible version. This PR updates compile.ts script to use the newer esbuild APIs. One difference between this and the previous version is that console.log('[${name}] build succeeded') will be called for every build, not only in watch mode.

Fixes #154

@vadimavdeev vadimavdeev changed the title Fix local builds on fresh installs Fix compilation on fresh installs Jul 12, 2023
Copy link
Member

@jodyheavener jodyheavener left a comment

Choose a reason for hiding this comment

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

Hey @vadimavdeev, thanks so much for uncovering and open a PR for this issue! I just tried to give it a spin and it appears I'm still getting an error (using yarn build and yarn watch):

$ node -r esbuild-register compile.ts
✘ [ERROR] Invalid option in context() call: "watch"

    /Repositories/op-vscode/node_modules/esbuild/lib/main.js:255:12:
      255 │       throw new Error(`Invalid option ${where}: ${quote(key)}`);
          ╵             ^

    at checkForInvalidFlags (/Repositories/op-vscode/node_modules/esbuild/lib/main.js:255:13)
    at flagsForBuildOptions (/Repositories/op-vscode/node_modules/esbuild/lib/main.js:457:3)
    at buildOrContextContinue (/Repositories/op-vscode/node_modules/esbuild/lib/main.js:1009:9)
    at /Repositories/op-vscode/node_modules/esbuild/lib/main.js:983:11
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

I'll be happy to take a closer look at this during my next cycle, but I wanted to raise in case you had any ideas. Are you also experiencing this?

@vadimavdeev
Copy link
Contributor Author

vadimavdeev commented Jul 18, 2023

Hey @vadimavdeev, thanks so much for uncovering and open a PR for this issue! I just tried to give it a spin and it appears I'm still getting an error (using yarn build and yarn watch):

$ node -r esbuild-register compile.ts
✘ [ERROR] Invalid option in context() call: "watch"

    /Repositories/op-vscode/node_modules/esbuild/lib/main.js:255:12:
      255 │       throw new Error(`Invalid option ${where}: ${quote(key)}`);
          ╵             ^

    at checkForInvalidFlags (/Repositories/op-vscode/node_modules/esbuild/lib/main.js:255:13)
    at flagsForBuildOptions (/Repositories/op-vscode/node_modules/esbuild/lib/main.js:457:3)
    at buildOrContextContinue (/Repositories/op-vscode/node_modules/esbuild/lib/main.js:1009:9)
    at /Repositories/op-vscode/node_modules/esbuild/lib/main.js:983:11
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

I'll be happy to take a closer look at this during my next cycle, but I wanted to raise in case you had any ideas. Are you also experiencing this?

Yep, my bad. watch: true slipped through when I tested the error handling code I put back. Should be fine now. Could you check again?

Copy link
Member

@jodyheavener jodyheavener left a comment

Choose a reason for hiding this comment

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

That did it!

@jodyheavener
Copy link
Member

jodyheavener commented Jul 18, 2023

Ah - apologies, commits going into main need to be signed. Possible for you to squash this work and sign the commit? If you're new to commit signing we've made it real easy to get set up 😏

@vadimavdeev
Copy link
Contributor Author

Ah - apologies, commits going into main need to be signed. Possible for you to squash this work and sign the commit? If you're new to commit signing we've made it real easy to get set up 😏

Sure, let me do that.

Fixes local builds not working due to esbuild being upgdaded to a
backward-incompatible version.
@vadimavdeev
Copy link
Contributor Author

Done. It was real easy indeed. 1Password's docs are excellent.

@jodyheavener
Copy link
Member

Awesome, thank you so much for fixing this!

@jodyheavener jodyheavener merged commit 7b3126b into 1Password:main Jul 19, 2023
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.

Development: Build fails on fresh installations
2 participants