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

Adding @netlify prefix broke the esbuild no-optional workaround #17

Closed
pimlie opened this issue May 6, 2022 · 8 comments · Fixed by #18
Closed

Adding @netlify prefix broke the esbuild no-optional workaround #17

pimlie opened this issue May 6, 2022 · 8 comments · Fixed by #18

Comments

@pimlie
Copy link

pimlie commented May 6, 2022

See the below build output. Adding the @netlify prefix seems to have broken the download path used by esbuild.

It should be
https://registry.npmjs.org/@netlify/esbuild-linux-64/-/esbuild-linux-64-0.14.23.tgz
and not
https://registry.npmjs.org/@netlify/esbuild-linux-64/-/@netlify/esbuild-linux-64-0.14.23.tgz

12:47:44 PM: error /opt/build/repo/node_modules/@netlify/esbuild: Command failed.
12:47:44 PM: Exit code: 1
12:47:44 PM: Command: node install.js
12:47:44 PM: Arguments:
12:47:44 PM: Directory: /opt/build/repo/node_modules/@netlify/esbuild
12:47:44 PM: Output:
12:47:44 PM: [esbuild] Failed to find package "@netlify/esbuild-linux-64" on the file system
12:47:44 PM: This can happen if you use the "--no-optional" flag. The "optionalDependencies"
12:47:44 PM: package.json feature is used by esbuild to install the correct binary executable
12:47:44 PM: for your current platform. This install script will now attempt to work around
12:47:44 PM: this. If that fails, you need to remove the "--no-optional" flag to use esbuild.
12:47:44 PM: [esbuild] Trying to install package "@netlify/esbuild-linux-64" using npm
12:47:44 PM: [esbuild] Failed to install package "@netlify/esbuild-linux-64" using npm: ENOENT: no such file or directory, rename '/opt/build/repo/node_modules/esbuild/lib/npm-install/node_modules/@netlify/esbuild-linux-64/bin/esbuild' -> '/opt/build/repo/node_modules/esbuild/lib/downloaded-@netlify/esbuild-linux-64-esbuild'
12:47:44 PM: [esbuild] Trying to download "https://registry.npmjs.org/@netlify/esbuild-linux-64/-/@netlify/esbuild-linux-64-0.14.23.tgz"
12:47:44 PM: [esbuild] Failed to download "https://registry.npmjs.org/@netlify/esbuild-linux-64/-/@netlify/esbuild-linux-64-0.14.23.tgz": Server responded with 404
12:47:44 PM: /opt/build/repo/node_modules/@netlify/esbuild/install.js:236
12:47:44 PM:         throw new Error(`Failed to install package "${pkg}"`);
12:47:44 PM:               ^
12:47:44 PM: Error: Failed to install package "@netlify/esbuild-linux-64"
12:47:44 PM:     at checkAndPreparePackage (/opt/build/repo/node_modules/@netlify/esbuild/install.js:236:15)
12:47:44 PM:     at processTicksAndRejections (node:internal/process/task_queues:96:5)
12:47:44 PM: info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
12:47:51 PM: Error during Yarn install
12:47:51 PM: Build was terminated: Build script returned non-zero exit code: 1
12:47:51 PM: Creating deploy upload records
@Skn0tt
Copy link

Skn0tt commented May 6, 2022

Hi! Can you add some reproductions steps? Running yarn add @netlify/esbuild works fine for me, but i'm on darwin-arm64. I assume you're running this on Linux?

@pimlie
Copy link
Author

pimlie commented May 6, 2022

Sorry, I missed a step in my report!

Locally everything works fine because I install the dependencies with yarn install

But in the Deploy Settings on Netlify I have set:

  YARN_FLAGS = "--frozen-lockfile --production --ignore-optional"

and then the build fails due to --ignore-optional

As workaround I have removed ignore-optional for now but this means that we are installing more dependencies then we need so build time could take a bit longer :)

@Skn0tt
Copy link

Skn0tt commented May 6, 2022

Interesting! Is @netlify/esbuild part of your custom build process, or does it fail as a transitive dependency of https://github.com/netlify/build?

@pimlie
Copy link
Author

pimlie commented May 6, 2022

Using yarn why we are using it because we use @netlify/zip-it-and-ship-it (which has a dependency on netlify/build indeed)

@Skn0tt
Copy link

Skn0tt commented May 6, 2022

Okay, that's good info! Thank you :) I'll take some time next week to look into this.

@Skn0tt
Copy link

Skn0tt commented May 6, 2022

okay, faster than expected ^^ I'll release this together with #16 once that's merged, will ping back to let you know.

@pimlie
Copy link
Author

pimlie commented May 6, 2022

Awesome, thats super quick indeed. Thanks! 🎉

@Skn0tt
Copy link

Skn0tt commented May 9, 2022

Published in v0.14.25. Can you check if that fixes it for you?

ericapisani added a commit to netlify/netlify-plugin-gatsby that referenced this issue May 24, 2022
package resolution wasn't picking up fix to netlify/esbuild#17
kodiakhq bot pushed a commit to netlify/netlify-plugin-gatsby that referenced this issue May 26, 2022
* feat: optionally load Gatsby datastore into lambdas after deploy

* chore: add logging to determine code path

* fix: dataMetadata.json file wasn't being included in function bundle

* fix: await promise returned from prepareFilesystem

Suspect that the function is completing early because this wasn't happening before

* fix: address 'handler is not a function' err

* fix: move prepareFilesystem check within returned handler

* chore: add logging

* fix: downloadUrl is incorrect

* fix: round 2 on the url

* fix: only copy file that we're interested in for mvp

* fix: create directory to copy file into before streaming

* fix: specify the full filepath

thought this would download the file into the directory, but suspecting that the file needs to exist
before the stream writes

* style: lint fixes

* feat: add pre-warm requests to lambda endpoints

aiming to mitigate the effects of the cold start when downloading the
gatsby datastore from the CDN

* fix: use https instead of fetch for request

* fix: mistake with the func path

* test: not seeing prewarm behaviour, add logs

* style: lint fixes

* refactor: rename e2e test folder

* test: update test command in package.json

* refactor: more organizing

* test: add tests for downloadFile helper method

* style: lint

* chore: add tsconfig.json

small cleanups

* chore: cleanup

* test: add createDatastoreMetadataFile test

* chore(test): rename file

* test: add onSuccess test

add chance.js as dependency for randomized values

* chore: make changes flagged by linter

* style: lint

* fix: explicitly check env var is 'true'

If LOAD_GATSBY_LMDB_DATASTORE_FROM_CDN were set to 'false',
the original conditional would still evaluate to 'true' when not cast as a boolean.

* test: add coverage for onBuild method

* style: lint changes

* style: lint fixes

* refactor: revert reorg of test files

Will do this separately in order to reduce amount of changes in this PR

* refactor: remove README.md from .gitignore

revert change to command to run e2e tests

* fix: check value in case it's set to 'false'

* test: add mutateConfig coverage

* test: revert accidental change to jest config

* test: add test for prepareFilesystem

* feat: add timeout to pre-warm requests

requires a minimum supported node version of 14.17 as
that's when AbortController was introduced globally

* style: lint

* test: update tests after merge

* style: lint

* ci: add gatsby to global install step

the demo project is being built in some tests and requires gatsby to be available

* ci: forgot to add gatsby dependency to ubuntu tests

* test: remove gatsby install

wasn't having desired effect. Try something else

* ci: revert change

* Apply suggestions from code review

Co-authored-by: Matt Kane <m@mk.gg>

* fix: address some code review comments

* fix: address some code review comments

* fix: address cr comments

move the dataMetadata.json file to the .cache directory rather than public.

* fix: don't generate unique id for datastore every build

reuse the existing ID if a datastore file already exists in the publish directory

* refactor: isEnvSet

* ci: install demo/ deps

Attempting to address missing gatsby error in test builds

* ci: fix silly mistake

* test: increase timeout

the tests currently failing on build are long running ones, and the Github
 workers might need more time than a local machine

* test: use netlify-cli to build project

tests were failing on github workers because the wrong command to build was being used

* ci: add install command back in

* fix: hard code env var check

* style: lint

* fix: address cr comments

add check for dataMetadata.json file in order to use filename there if it exists

* fix: regenerate package-lock to fix conflicts

* fix: regenerate package-lock

* chore: update @netlify/build resolution

package resolution wasn't picking up fix to netlify/esbuild#17

* chore: run npm update on plugin/

* chore: check in working version of plugin lockfile

* chore: lockfile

* chore: add lockfile back in

* chore: update lint rules

increase version of node to 14

* chore: pin eslint-config-node

* test: add axios to invalidate the github test cache

* chore: increase node version

remove axios as well

* refactor: attempt to read metadata file first

this avoids needing to traverse the list of files to reuse the name when possible

* chore(deps): move execa to devDep in plugin

* style: lint

Co-authored-by: Matt Kane <m@mk.gg>
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 a pull request may close this issue.

2 participants