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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test to fs.readFileSync in TypeScript files (#1736) #2046

Merged

Conversation

hasparus
Copy link
Contributor

@hasparus hasparus commented Sep 23, 2018


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:

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 #1736.

鉁旓笍 PR Todo

@DeMoorJasper DeMoorJasper merged commit 5fdb722 into parcel-bundler:master Sep 25, 2018
@DeMoorJasper
Copy link
Member

It's good to have extra tests, but it would probably be better if we fixed the actual issue you had with it. At least if it's supported in normal JS

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Sep 25, 2018

About codesandbox, it's probably using an outdated version of parcel and it has also been modified (probably quite a lot) to work in the browser as far as I know.

devongovett pushed a commit that referenced this pull request Oct 15, 2018
---
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 #1736.

## 鉁旓笍 PR Todo
- [X] Link in #1736.
- [X] Post reproduction to CodeSandbox.
- [ ] Document how to use `readFileSync` in TypeScript?
devongovett pushed a commit that referenced this pull request Oct 15, 2018
---
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 #1736.

## 鉁旓笍 PR Todo
- [X] Link in #1736.
- [X] Post reproduction to CodeSandbox.
- [ ] Document how to use `readFileSync` in TypeScript?
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 this pull request may close these issues.

None yet

2 participants