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

[WIP] Fix HMR failure with js error on load #898

Closed
wants to merge 693 commits into from

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Feb 26, 2018

global.require isn't defined if a childBundle called by newRequire throws an error.
This causes an error on HMR update.

More details in #879 (comment)

fixes #879

@mischnic
Copy link
Member Author

mischnic commented Feb 26, 2018

Tests fail because errors from require don't bubble up anymore.

  try {
    // We expect browser-resolve to replace fs with an empty module, so readFileSync will be undefined
    return require('fs').readFileSync(__dirname + '/package.json')
  }
  catch(_) {
    return 'test-pkg-ignore-fs-ok'
  }

Would it be better to instead change prelude.js to something like this: (this makes sure require isn't undefined)?

require = function (...){...};
require({1:[], .....});

(Maybe this doesn't work at all because of how prelude.js works)

@mischnic mischnic changed the title Fix HMR failure with js error on load [WIP] Fix HMR failure with js error on load Feb 26, 2018
@davidnagli davidnagli added the 📝 WIP Work In Progress label Feb 26, 2018
@wzrdtales
Copy link

loocking forward to this

@wzrdtales
Copy link

@mischnic localRequire needs the same fix, since it bares the same problem

@mischnic
Copy link
Member Author

mischnic commented Mar 9, 2018

@devongovett Could you take a look at how to fix this correctly?

devongovett and others added 24 commits March 20, 2018 19:46
Now that this works correctly in the dev server as of parcel-bundler#974, this should be a much more sensible default for both production and development builds. Fixes parcel-bundler#714.
@FDiskas
Copy link

FDiskas commented Sep 25, 2018

📨 waiting for the fix also

hasparus and others added 8 commits September 25, 2018 13:45
…parcel-bundler#2046)

---
name: Add test to fs.readFileSync in TypeScript files
---
```
parcel-bundler version 1.9.7
```

## 💥 Problem

Inlining file in browser mode doesn't work in TypeScript files when `fs` is imported with star import or default import. The only way to use `readFileSync` is:
```typescript
import { readFileSync } from 'fs';
const raw = readFileSync(__dirname + '/raw.tsx', 'utf-8');
```

### Reproduction: 
Repo: https://github.com/hasparus/parcel-readfilesync-typescript-repro
ghPages: https://hasparus.github.io/parcel-readfilesync-typescript-repro/
CodeSandbox: https://codesandbox.io/s/github/hasparus/parcel-readfilesync-typescript-repro/tree/master/

Fun Fact: All ways to import `fs` work on CodeSandbox (parcel template).

## ↪️ Pull Request
Adds test to `fs.readFileSync` in TypeScript files and thus provides better workaround for parcel-bundler#1736.

## ✔️ PR Todo
- [X] Link in parcel-bundler#1736.
- [X] Post reproduction to CodeSandbox.
- [ ] Document how to use `readFileSync` in TypeScript?
Was running `parcel build` and the auto installer started running, I went to disable it and found that the option was not available here
…dler#2089)

# ↪️ Pull Request

Do not load existing sourcemaps if sourcemaps are disabled
---
name: 🙋 Fix HMR for Pug assets
---

<!---
Thanks for filing a pull request 😄 ! Before you submit, please read the following:

Search open/closed issues before submitting since someone might have pushed the same thing before!
-->
## ↪️ Pull Request
<!---
Provide a general summary of the pull request here
Does this address an existing issue?
-->
parcel-bundler#2090

## 💻 Examples

<!-- Examples help us understand the requested feature better -->

## ✔️ PR Todo

- [ ] Added/updated unit tests for this change
- [ ] Filled out test instructions
- [x] Included links to related issues/PRs
<!-- Love parcel? Please consider supporting our collective:
👉  https://opencollective.com/parcel/donate -->
@devongovett
Copy link
Member

Due to move to monorepo which rewrote the git history, this PR was automatically closed. Please open a new PR against master with your changes. You can do a diff against the 1.x branch to see them. Sorry about that!

@mischnic mischnic deleted the hmr_error_fix branch January 8, 2019 17:08
@mischnic mischnic restored the hmr_error_fix branch January 8, 2019 17:08
@mischnic mischnic deleted the hmr_error_fix branch January 11, 2019 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug HMR Hot Module Reloading 📝 WIP Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛Uncaught Error: Cannot find module '21'