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

Use tokenizer browser #268

Merged
merged 1 commit into from
Dec 22, 2019
Merged

Use tokenizer browser #268

merged 1 commit into from
Dec 22, 2019

Conversation

Borewit
Copy link
Collaborator

@Borewit Borewit commented Dec 6, 2019

Part of implementation of #248

Add browser specific functions.

Get file type based on content from Web API ReadableStream:

const FileType = require('file-type/browser');
(async () => {

    const response = await fetch("https://www.example.org/");
    const fileType = await FileType.fromStream(response.body);
})();

Get file type based on content from Blob:

const FileType = require('file-type/browser');
(async () => {
    const fileType = await FileType.fromBlob(blob);
})();

browser.js Outdated Show resolved Hide resolved
browser.js Outdated Show resolved Hide resolved
browser.js Outdated Show resolved Hide resolved
browser.js Outdated Show resolved Hide resolved
@Borewit
Copy link
Collaborator Author

Borewit commented Dec 20, 2019

Do you mind merging #261 & #262 first @sindresorhus? I am getting a little bit dizzy with all those outstanding PR's. Thanks a lot a for your detailed feedback, I know reviewing reviewing is lot of work.

@sindresorhus
Copy link
Owner

Done :)

@Borewit Borewit changed the base branch from master to stream-tokenizer December 21, 2019 12:42
@Borewit
Copy link
Collaborator Author

Borewit commented Dec 21, 2019

Okay, I cleaned up a bit.

I created to small PR with some fixing changes already merge (to prevent things getting mixed up):

Than there is #265 preceding this PR, which move most functionality from index.js to core.js, which make core.js a polymorphic core (can be used by Node.js & browser). No rocket science, it essentially prevents a dependency on fs which is pretty Node.js specific.

At last there is this PR, #268, which adds browser functions and corresponding dependencies. If you change your mind, as the change is more clear, and prefer moving to a separate module, this is a good escape point.
Yet another module to maintain, at the other hand it maybe easier to add browser unit tests and prevent browser specific dependencies to end up in the Node dependency tree.

@sindresorhus
Copy link
Owner

This looks good too when you fix the merge conflict.

@sindresorhus sindresorhus merged commit 0277f45 into sindresorhus:stream-tokenizer Dec 22, 2019
This was referenced Jan 1, 2020
@Borewit Borewit mentioned this pull request Feb 9, 2020
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