Skip to content

react starter needs node ref, accept fixed leveldown#19994

Merged
weswigham merged 1 commit into
microsoft:masterfrom
weswigham:accept-new-user-baselines
Nov 14, 2017
Merged

react starter needs node ref, accept fixed leveldown#19994
weswigham merged 1 commit into
microsoft:masterfrom
weswigham:accept-new-user-baselines

Conversation

@weswigham
Copy link
Copy Markdown
Member

No description provided.

@weswigham weswigham requested a review from sandersn November 13, 2017 23:20
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 14, 2017

seems wrong that you need react.d.ts and node.d.ts in the same compilation.. does leveldown really needs node?

@weswigham
Copy link
Copy Markdown
Member Author

weswigham commented Nov 14, 2017

@mhegazy not leveldown, but the react starter; specifically because it uses require as a function. That was just an oversight in adding the test, I think, since it had type errors before, as the starter does have @types/node in its package.json.

@weswigham weswigham merged commit 9be4d60 into microsoft:master Nov 14, 2017
@weswigham weswigham deleted the accept-new-user-baselines branch November 14, 2017 00:44
@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 14, 2017

leveldown seems to rely on node as well..

for require, requireJs should have been used instead..

Just to be clear, the test as you have now is fine, i am just noting an issue with the starter kit.

@sandersn
Copy link
Copy Markdown
Member

Note to self: leveldown still failed for me until I deleted user/leveldown/node_modules. user tests might need to delete node_modules too.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants