Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Sapper does not set ROLLUP_WATCH environment variable #1301

Closed
benmccann opened this issue Jul 2, 2020 · 2 comments
Closed

Sapper does not set ROLLUP_WATCH environment variable #1301

benmccann opened this issue Jul 2, 2020 · 2 comments

Comments

@benmccann
Copy link
Member

Describe the bug
Sapper does not set the ROLLUP_WATCH environment variable that Rollup sets when in watch mode:

https://github.com/rollup/rollup/blob/78f95f63c077939a12ba5f5511bca15d4af94443/cli/run/watch-cli.ts#L19

The TypeScript plugin relies on ROLLUP_WATCH being set and will not work properly in watch mode without it

To Reproduce

Steps:

  1. Clone and install https://github.com/babichjacob/sapper-ts-mismatch
  2. Run npm run dev
  3. Make a GET request, such as by visiting in the browser, /fails and see the result 1
  4. Modify src/routes/fails.ts and change 1 to 2. The change will be picked up and "rebuild"
  5. Do 3 again but still see 1 instead of the new 2

Expected behavior
Watching should pick up changes

Information about your Sapper Installation:

  • Sapper version: 0.27.16
  • Rollup 2
  • Rollup TypeScript plugin 4.1.2 and newer

Severity
Highly annoying but not a blocker

Additional context

This is one of many bugs cause by trying to reimplement Rollup's CLI functionality, but failing to do so in the same exact manner that Rollup does. Some of these bugs have been fixed and some have not. E.g. #1221 #1234 #1266

I wonder if it might be better to make Sapper's code generator be a Rollup plugin instead

@dionysiusmarquis
Copy link

dionysiusmarquis commented Jul 2, 2020

It just wasn‘t possible for rollup plugins to implement watch specific logic in rollup.watch context until rollup@^2.14.0. There is rollup/plugins#449 for the Typescript plugin pending that will fix this by updating the rollup peer dependency to a rollup version that has this ability and implements the new watch context check (without process.env.ROLLUP_WATCH). Adding process.env.ROLLUP_WATCH = true will let the plugin “think” its in rollup -w cli context. This will fix the issue for the moment, but it is more a quick and “hacky” workaround to get it working before the rollup/plugins#449 is merged (you will still need @rollup/plugin-typescript@^5.0.1).

@benmccann
Copy link
Member Author

Ah, thanks for the additional details. I'll go ahead and close this then and wait for your PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants