Skip to content

Conversation

JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Jul 10, 2018

I held off breaking API changes such as moving Webpack from 1 to 4 (!) out of laziness.

Fixes #566.

Edit: Err, Map is not defined build errors. Reducing scope to just React.

Josh Goldberg added 2 commits July 9, 2018 18:43
I held off breaking API changes such as moving Webpack from 1 to 4 (!) out of laziness.

Fixes #566.
@JoshuaKGoldberg JoshuaKGoldberg changed the title Bumped to latest minor versions in React.Core package.json Bumped to latest minor React versions in React.Core package.json Jul 10, 2018
@JoshuaKGoldberg
Copy link
Contributor Author

ReferenceError: Map is not defined [C:\projects\react-net\src\React.Sample.Mvc4\React.Sample.Mvc4.csproj]
C:\projects\react-net\src\React.Sample.Mvc4\TransformBabel.proj(5,3): error MSB4018: at React.Core.Resources.react.generated.min.js:4:69

I don't see why this is happening... no plans to investigate on my end, but I'll leave the PR open for visibility. Sorry :)

@dustinsoftware
Copy link
Member

We should update the webpack samples before merging this.

@dustinsoftware
Copy link
Member

Updating to React 16.4 breaks at least one JS engine that is loaded by default (VroomJsEngine). Need to audit the samples and fix any failures before merging.

@Taritsyn
Copy link
Contributor

@dustinsoftware The VroomJs library uses a very old version of V8. Version 3.17.16.2 was released more than 5 years ago (April 11, 2013).

@dustinsoftware
Copy link
Member

dustinsoftware commented Jul 23, 2018 via email

@Taritsyn
Copy link
Contributor

@dustinsoftware If you're talking about this PR, then I don't like the following block of source code:

///<summary>
/// Gets or sets the name of the default JsEngineSwitcher engine.
/// </summary>
public string DefaultEngineName
{
	get
	{
		return JsEngineSwitcher.Instance.DefaultEngineName;
	}
	set
	{
		JsEngineSwitcher.Instance.DefaultEngineName = value;
	}
}

It is not safe to change the DefaultEngineName property of the JavaScript Engine Switcher, which other libraries can use, from the ReactJS.NET. I would name this property EngineName and set the default value to an empty string. In case the user did not set a value to this property, I would try to take the value from the JsEngineSwitcher.Instance.DefaultEngineName. If it is also empty, then I would throw an exception.

@dustinsoftware
Copy link
Member

Fixed in fe72ea9

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.

3 participants