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

Upgrade to latest Netlify build API #18

Merged
merged 1 commit into from
Mar 31, 2020
Merged

Upgrade to latest Netlify build API #18

merged 1 commit into from
Mar 31, 2020

Conversation

ehmicky
Copy link
Contributor

@ehmicky ehmicky commented Mar 30, 2020

Thanks a lot for creating this Build plugin!

Netlify just released the latest version of Build plugins. Build plugins are currently enabled in the Netlify website for a small percentage of beta users.

This PR upgrades this plugin to the latest API. You can find the new documentation here.

I don't have a Cypress setup, so you definitely would want to manually test the different changes introduced by this PR.

List of the changes:

  • Added keywords to package.json to make it easier for users to find your plugin
  • Added a bugs keyword to package.json which will be printed to users if a bug happens inside your plugin
  • Replace execa with Netlify's run utility, which is based on Execa (note: I am a co-maintainer of Execa)
  • When pings on wait-on fail, use utils.build.failPlugin() (which will be reported as a user error) instead of throwing an error (which would be reported as a bug inside your plugin)
  • On errors, use utils.build.failPlugin() instead of utils.build.failBuild() to ensure other plugins keep running.
  • Avoid plugin's top-level function since this might be deprecated in the future and is not currently used by this plugin
  • Remove the name property, which has been removed from Netlify
  • The preBuild and postBuild events have been renamed to onPreBuild and onPostBuild
  • Use the PUBLISH_DIR constant instead of netlifyConfig.build.publish. This constant not only uses the configuration file property build.publish but also the setting "Publish directory" set in the Netlify app.
  • pluginConfig was renamed to inputs
  • Remove return value from event handlers, since those are reserved for future usage

Note: checking that utils.build exists should not be needed in production. The only way this would not exist is for users running a local build with an old version Netlify CLI, but upgrading should fix their problem.

Those are quite many changes, so if you find any time to confirm that each works by running actual builds, this would be amazing! Thanks. 🙏

@CLAassistant
Copy link

CLAassistant commented Mar 30, 2020

CLA assistant check
All committers have signed the CLA.

@ehmicky ehmicky mentioned this pull request Mar 30, 2020
23 tasks
@bahmutov bahmutov self-requested a review March 31, 2020 03:36
@bahmutov
Copy link
Contributor

Looks good, let me try this tomorrow before publishing. I think this does not even have user-facing changes, so could be even a patch version update.

@bahmutov
Copy link
Contributor

actually, steps were renamed, so user would have to use onPreBuild - but this early in the game, I guess a minor version is still fine

@ehmicky
Copy link
Contributor Author

ehmicky commented Mar 31, 2020

Oh yes, you're right. You could either make a breaking change and support only passing the new event name, or keep backward compatibility there.

Please let me know if you have any additional questions, I'm here to help! :)

Copy link
Contributor

@bahmutov bahmutov left a comment

Choose a reason for hiding this comment

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

Looks good, I will merge this so I release a beta version to test it out, then will set up CI tests before publishing an official version, thanks for the work @ehmicky I really appreciate this

@bahmutov bahmutov changed the title Upgrade to latest Netlify Upgrade to latest Netlify build API Mar 31, 2020
@bahmutov bahmutov merged commit d777e3f into cypress-io:master Mar 31, 2020
@bahmutov
Copy link
Contributor

@ehmicky do the users still need to specify inputs to onPreBuild callback as [plugins.inputs.preBuild] or should it now be [plugins.inputs.onPreBuild]?

@bahmutov
Copy link
Contributor

also @ehmicky I have noticed the pre-build callback failing, yet the build has succeeded? https://circleci.com/gh/cypress-io/netlify-plugin-cypress/96

Screen Shot 2020-03-31 at 10 30 29 AM

bahmutov added a commit that referenced this pull request Mar 31, 2020
- this is a fix because under the hood it updates this plugin to latest built API (work was done in PR #18)
@bahmutov
Copy link
Contributor

🎉 This PR is included in version 1.3.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ehmicky
Copy link
Contributor Author

ehmicky commented Mar 31, 2020

@ehmicky do the users still need to specify inputs to onPreBuild callback as [plugins.inputs.preBuild] or should it now be [plugins.inputs.onPreBuild]?

I kept it as inputs.preBuild but providing the new name (either as a breaking change or backward compatible) might be a good idea?

I have noticed the pre-build callback failing, yet the build has succeeded?

This PR switched from using utils.build.failBuild() (which makes whole build fail) to using utils.build.failPlugin() (which makes only this specific plugin fail). Which one you prefer to use depends on how strict/lenient your plugin should be when it comes to the whole build. Netlify favors being lenient but surfacing the errors in the UI (which is currently a WIP). However if you think the Cypress should be stricter, failBuild() should be used instead 👍

@ehmicky ehmicky deleted the feat/upgrade-netlify branch March 31, 2020 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants