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

yarn build and the project contains the entire source code #1062

Closed
neilyoung opened this issue Mar 11, 2019 · 46 comments
Closed

yarn build and the project contains the entire source code #1062

neilyoung opened this issue Mar 11, 2019 · 46 comments

Comments

@neilyoung
Copy link
Contributor

neilyoung commented Mar 11, 2019

I came across this accidently. In the past I was used to have an obfuscated version of my SPA after "npm run build".

With the latest react-static (which does not work correctly with npm run build, but it passes with yarn build, see here #1053) there is the full source code available in user's browser. In the beginning I thought, this is just because there is somewhere a file link to the source code in a map or so and it would be only visible to me, but after deploying it to a webserver it was also available to all.

Steps to reproduce:

  • react-static create
  • use "blank" target
  • To visualize the issue add a console.log("Hello World") in App.js, but you could also browse the source tree in developer console
render () {
    console.log("Hello World")
    return (
  • run yarn build
  • run `yarn serve``
  • open localhost:3000 in your browser
  • open developer console
  • find Hello World and click the right most link to 'App.js:9'

Find your sources. Bummer

I never had this with previous react-static versions.

@tannerlinsley
Copy link
Contributor

tannerlinsley commented Mar 11, 2019

Yeah, this definitely is a problem. I'm not sure how it's happening, but it could be that source maps are not being configured properly for production builds. Would you be willing to look into this more?

@neilyoung
Copy link
Contributor Author

neilyoung commented Mar 12, 2019

Sorry for the late answer, wasn't watching. Yes, sure, if I can help, let's try something. But I need some kicks, things like this is not my daily doing..

@tannerlinsley
Copy link
Contributor

tannerlinsley commented Mar 12, 2019

After doing a search in the basic template on my machine for my computer usernametannerlinsley, this is the preview. So at least we know the files to look in...

Screen Shot 2019-03-11 at 10 16 10 PM

@neilyoung
Copy link
Contributor Author

Hmm. This is the entire contents of a new folder which sprang into my eyes for the very first time I dealt with 6.9.3 and which is not there in earlier versions. The directory is called artifacts at the same level as node_modules and dist and is created by a yarn build and yarn start

__Documents_test_spa_—_-bash_—_150×30_und___Documents_test_spa_—_-bash_—_150×30

Interestingly enough during yarn build there is a console trace, which claims, the folder is about to be cleaned, but this seems to not happen or is reverted later on.

__Documents_test_spa_—_-bash_—_150×30

Good catches so far. 👍

@neilyoung
Copy link
Contributor Author

@neilyoung
Copy link
Contributor Author

I stepped back a couple of versions. From 5.9.7 (this was my first step back from 6.3.9, but it still had all the sourecode in webpack build) to now 4.6.2

Namely the core dependencies look so:

    "axios": "^0.16.2",
    "react": "^16.0.2",
    "react-dom": "^16.2.0",
    "react-router": "^4.2.0",
    "react-static": "^4.6.0",

With this setup there is a problem with the hotloader, so I commented it for now and will return to the old style hotloading later.

But in the end npm run build worked, no artifacts folder and npm run serve without source code

@neilyoung
Copy link
Contributor Author

@tannerlinsley Wondering, why this issue does not have more attention. I would say nobody would like to see his source code on the web while thinking it is at least obfuscated...

@tannerlinsley
Copy link
Contributor

It's at the top of my security/priority list for React Static. I appreciate your patience as we all work to figure this out.

@neilyoung
Copy link
Contributor Author

@tannerlinsley Thanks. Take your time. I'm having a work around so far

@tannerlinsley
Copy link
Contributor

It appears that source maps are being created for non-staging production builds. While this should be a togglable feature, this is a regression that should not be the default. We'll need to tweak this setting here: https://github.com/nozzle/react-static/blob/master/packages/react-static/src/static/webpack/webpack.config.prod.js#L90.

For react-static build --staging builds, we'll likely want to have this default to true and for production builds, default to false. It should also be configurable somehow in the future, but is not very important right now.

@neilyoung
Copy link
Contributor Author

@tannerlinsley Makes sense. Anything to do for me?

@tannerlinsley
Copy link
Contributor

You could create a PR that does what I outlined there. Then we can iterate on it, get it merged and cut a release asap.

@neilyoung
Copy link
Contributor Author

neilyoung commented Mar 13, 2019 via email

@tannerlinsley
Copy link
Contributor

An easy way to verify: confirm that files like dist/main.js.map are not being created.

@neilyoung
Copy link
Contributor Author

OK, on duty

@neilyoung
Copy link
Contributor Author

@tannerlinsley No such a PR wouldn't help. I have patched the flag in my copy of the script at line 90 to sourceMap=false. Nevertheless there is an artifact dir on a fresh project and yarn build also produces souremaps in dist.

@tannerlinsley
Copy link
Contributor

Oh, sorry, there is one more that needs to be taken care of. https://github.com/nozzle/react-static/blob/master/packages/react-static/src/static/webpack/webpack.config.prod.js#L128 should be set appropriately as well.

@neilyoung
Copy link
Contributor Author

Hmm.. and what would be a proper value here?

@tannerlinsley
Copy link
Contributor

Similar to my prior statement, it should default to true during a staging build and default to false in a production build.

@neilyoung
Copy link
Contributor Author

You mean

devtool: false,

instead of

devtool: 'source-map',

?

@neilyoung
Copy link
Contributor Author

No, doesn't help. I'm out here :)

@tannerlinsley
Copy link
Contributor

tannerlinsley commented Mar 13, 2019

Curious how are you testing these changes locally?

@tannerlinsley
Copy link
Contributor

Just want to make sure we're being thorough.

@neilyoung
Copy link
Contributor Author

neilyoung commented Mar 13, 2019 via email

@tannerlinsley
Copy link
Contributor

If you're altering src files, then you'll need to compile first. Or, you could alter the same file but in the lib directory.

@neilyoung
Copy link
Contributor Author

Oops, stupid me. Altered in lib folder, to no avail

@neilyoung
Copy link
Contributor Author

Oh wait. artifacts is still there, but no maps... testing

@neilyoung
Copy link
Contributor Author

Yepp. Looks good. Will try to make a PR

@tannerlinsley
Copy link
Contributor

Awesome. Thanks!

@neilyoung
Copy link
Contributor Author

Done #1067

@digitalkaoz
Copy link
Contributor

@tannerlinsley i prefer to have sourcemaps for my whole app. dont disable that, they are only loaded when the dev tools are open, they are not included in the bundles

@neilyoung
Copy link
Contributor Author

I'm not fanatic on this. I could live with a config switch, which prevent the sourceMap deployment in release builds

@digitalkaoz
Copy link
Contributor

digitalkaoz commented Mar 26, 2019 via email

@tannerlinsley
Copy link
Contributor

We'll have a config option for this sourceMaps: true and possibly some other options. This option will likely default to false, given that most people don't need source maps in production.

Yes, you could easily just not upload them, but I think that would provide more trouble for people who repeatedly have to remove them or exclude them from the build directory, etc. While that's still possible, I think the flag will be more intuitive for both use cases.

@neilyoung
Copy link
Contributor Author

@tannerlinsley I don't currently have it on top of my head: Where is that config option?

And yes, simply don't upload would solve it too :)

@neilyoung
Copy link
Contributor Author

neilyoung commented Mar 26, 2019

Simply dont Upload the .map files. Its Up to you

True. The problem was: I'm using a particular react-static app in a Java app. The entire build process finally just triggered npm run build and made a copy of the dist directory into the Java app before deployment. Out of the sudden a react-static update gave me the source maps. I simply just didn't notice... That was unlucky, indeed.

@tannerlinsley
Copy link
Contributor

It's not there yet. We haven't added it yet. We're discussing it right now :)

@neilyoung
Copy link
Contributor Author

Ah ok, sorry. Got that wrong. Confused meanwhile :)

@digitalkaoz
Copy link
Contributor

digitalkaoz commented Mar 26, 2019 via email

@neilyoung
Copy link
Contributor Author

Well yes and no. It is there but I try to make my best to make it as unreadable as possible. OK now?

@digitalkaoz
Copy link
Contributor

digitalkaoz commented Mar 26, 2019 via email

@tannerlinsley
Copy link
Contributor

@digitalkaoz, @neilyoung is using an obfuscator to keep some of his code as mangled as possible to deter copying and/or theft.

While I agree that any IP should technically be kept behind a server for security, it should be possible to remove sourcemaps and obfuscate code. The obfuscation is not React Static's responsibility, since this goes beyond the call of duty for the library, but it's merely a webpack extension to achieve it.

@neilyoung
Copy link
Contributor Author

I'm not just minimizing. I'm also obfuscating. I don't have API keys in, but I also don't want to give anything away for nothing. This is not an open source project. Anyway, there is also not that much secret in the JS code, since it serves a well documented API in Java Spark. But you never know, who comes around and says he has seen it all... I just want to distract the average user to look behind the curtain. Whoever is able to make this readable code, has deserved it, shall take his an be damned :)

If that isn't opportun, than I don't know what arguments would convince you

@neilyoung
Copy link
Contributor Author

Ah and yes, it is for sure bad code :)

@tannerlinsley
Copy link
Contributor

This conversation is likely to get off topic past what has been discussed. If you guys have anything else that pertains to the technical implementation of this feature, let me know, otherwise, I'll consider this issue ready to close as soon as we release the code.

@neilyoung
Copy link
Contributor Author

Right, feel free to close. I'm having my solutions

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

No branches or pull requests

3 participants