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

[Bug] Prestart scripts no longer run #995

Closed
1 task done
configurator opened this issue Feb 25, 2020 · 6 comments · Fixed by #1002
Closed
1 task done

[Bug] Prestart scripts no longer run #995

configurator opened this issue Feb 25, 2020 · 6 comments · Fixed by #1002
Labels
bug Something isn't working reproducible This issue can be successfully reproduced

Comments

@configurator
Copy link

configurator commented Feb 25, 2020

  • I'd be willing to implement a fix

Describe the bug

In both npm and yarn legacy, when running a script named xxx, if a script named prexxx exists it would be run beforehand. Likewise, a script named postxxx would be run afterwards.

In yarn berry, this is no longer the case; prestart, poststart no longer work for custom script names.

To Reproduce

{
  "name": "test-package",
  "scripts": {
    "prestart": "echo prestart",
    "start": "echo start"
  }
}

Screenshots

npm and yarn 1 work correctly:
image

yarn berry works incorrectly:
image

Sherlock repro

await packageJsonAndInstall({
  "scripts": {
    "prestart": "echo prestart",
    "start": "echo start"
  }
});
await expect(await yarn(`start`)).toContain(`prestart`);
@configurator configurator added the bug Something isn't working label Feb 25, 2020
@yarnbot yarnbot added the reproducible This issue can be successfully reproduced label Feb 25, 2020
@yarnbot

This comment has been minimized.

2 similar comments
@yarnbot

This comment has been minimized.

@yarnbot
Copy link
Collaborator

yarnbot commented Feb 26, 2020

This issue reproduces on master:

Error: expect(received).toContain(expected) // indexOf

Expected substring: "prestart"
Received string:    "start
"
    at module.exports (evalmachine.<anonymous>:8:35)
    at processTicksAndRejections (internal/process/task_queues.js:94:5)
    at async /github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:13
    at async executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:16)
    at async Object.executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:12)
    at async ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-1.0.38-d4f5e2dbf3-2.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:25:38)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-2.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-2.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-2.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)

@arcanis
Copy link
Member

arcanis commented Feb 26, 2020

We should add a note to https://yarnpkg.com/advanced/lifecycle-scripts about this - the gist is that pre and post scripts have been deliberately deprecated.

The reason for this is that they make the code execution implicit rather than explicit (so just looking at the script entry isn't enough to know what's getting executed), and that they cause awkward executions in other circumstances (for example, a script named serve will also cause preserve to be executed). Finally, they had inconsistent timing issues.

Note that this is purely about user scripts - we still support some pre and post scripts as long as they have a clearly defined semantic, such as preinstall / postinstall, or prepack and postpack.

@arcanis arcanis closed this as completed Feb 26, 2020
@configurator
Copy link
Author

This should also be specified in the migration guide, as it is an incredibly large difference from how things used to work, and will break many user scripts.

@arcanis
Copy link
Member

arcanis commented Feb 27, 2020

Now documented as of #1002

nweldev pushed a commit to fullwebdev/fullwebdev that referenced this issue Dec 3, 2022
yarn berry doesn't support those scripts on purpose

see yarnpkg/berry#995
nweldev pushed a commit to fullwebdev/fullwebdev that referenced this issue Dec 3, 2022
yarn berry doesn't support those scripts on purpose

see yarnpkg/berry#995
nweldev pushed a commit to fullwebdev/fullwebdev that referenced this issue Dec 3, 2022
yarn berry doesn't support those scripts on purpose

see yarnpkg/berry#995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working reproducible This issue can be successfully reproduced
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants