Skip to content

Allow add multiple entry points 1084 #8249

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

danvc
Copy link

@danvc danvc commented Dec 28, 2019

Hi reviewer.

The intention of this PR is to provide a solution for the following (3 years old) issue:
#1084

The implementation doesn't require eject process.

The instruction to test it is:

  1. Create your .html and .js file
  2. Declare a new page in the property appPages in your package.json file. The structure must follow this shape:
    {
      "name": "login",
      "title": "login",
      "appHtml": "public/#.html",
      "appIndexJs": "src/#"
    },
  1. Access it by changing the URL according the page. For example: `http://localhost:3000/#.html'

You can see it running on the following gif

React multiple pages

Thank you so much by giving me this chance.

@danvc danvc force-pushed the all_add_multiple_entry_points_1084 branch from 3ad38b5 to 306533f Compare December 28, 2019 22:25
@unrevised6419
Copy link

unrevised6419 commented Dec 30, 2019

@DanZeuss I would mark this PR as a still Work In Progress (WIP). There are more things to check, that will work with this

  • webpack-dev-server (each page under own pathname?)
  • Service worker (same service worker or no?)
  • Public assets (css, js, static assets)
  • PUBLIC_URL / homepage (common PUBLIC_URL, or split by pathname)
  • Build folder (kind of public assets)
  • App eject

@danvc
Copy link
Author

danvc commented Dec 30, 2019

@iamandrewluca

You could run yarn create-react-app your_test_app in order to perform these tests inside your_test_app, which includes running eject, build, start.

I'm even using a project created from this fork using yarn create-react-app. The eject, start and build works fine though.

@bretep
Copy link

bretep commented Dec 30, 2019

@iamandrewluca and @DanZeuss Who is assigned to test and verify these things work?

@unrevised6419
Copy link

@bretep Right now nobody is assigned. @DanZeuss should do some tests to check if everything is fine. Well at least current tests should pass 🙂 I'll find some time to do some tests this week with this new feature in progress.

@danvc
Copy link
Author

danvc commented Dec 30, 2019

@iamandrewluca and @DanZeuss Who is assigned to test and verify these things work?

Hey Bret. I'm not sure how the testing process works on this project. We could be working together in order to get it done.

@danvc
Copy link
Author

danvc commented Dec 30, 2019

@DanZeuss I would mark this PR as a still Work In Progress (WIP). There are more things to check, that will work with this

  • ✅ webpack-dev-server (each page under own pathname?)
  • Service worker (same service worker or no?)
  • ✅ Public assets (css, js, static assets)
  • ✅ PUBLIC_URL / homepage (common PUBLIC_URL, or split by pathname)
  • ✅ Build folder (kind of public assets)
  • ✅ App eject

There's just one missing feature to be tested. I'm going to test if the PWA is working fine on my mobile.

@unrevised6419
Copy link

unrevised6419 commented Dec 30, 2019

webpack-dev-server (each page under own pathname?)

I think that having login.html in url is not such a good idea.
While working on #7259 I think would be better to have something like

  • /
  • /#/
  • /admin/
  • /client/
  • /any-other-pathname/

@bretep
Copy link

bretep commented Dec 30, 2019

You make a fair point.

I personally use the HTML file and point my proxy to it as not to interfere with routes. Because this is static content, I think it is reasonable to have .html file vs. separate paths. If a user wants multiple entry points to work, they’re probably already using a reverse proxy.

My static content live in google cloud storage and I point my proxy directly at the index.html file and my multiple entry point index files. I had to do this regardless of multiple entry else routes would not work.

@bretep
Copy link

bretep commented Dec 30, 2019

Also, I think having separate paths is going to:

  1. Complicate the webpack and static content reference.
    As it is now, all the assets stay in the same path and each entry point has a unique asset.

  2. Setup instructions would also become more complicated.

  3. Debugging would be more complicated.

Keep it simple with another .html file and the SPA will work as expected, no special case. Let all the customization of domain or path happen in the proxy. If someone is creating multiple entries, then they are already doing more advance things and will be using a proxy. For someone cloning an app or tinkering around, this needs to work without playing with the routes. They can goto login.html in that case.

@bretep
Copy link

bretep commented Dec 31, 2019

Also I currently use PUBLIC_URL with my multiple entry and index files:

Line from my build command:
PUBLIC_URL=\"/version/$(git log -n 1 --oneline --format=%h .)/\"

@bretep
Copy link

bretep commented Dec 31, 2019

Example k8s nginx-ingress manifest (replace index.html with new_entry.html) if I wanted the new_entry under a path like /newEntry/ I would handle the logic here.

apiVersion: "extensions/v1beta1"
kind: "Ingress"
metadata: 
  annotations: 
    acme.cert-manager.io/http01-edit-in-place: "true"
    cert-manager.io/cluster-issuer: "letsencrypt"
    kubernetes.io/ingress.class: "nginx"
    kubernetes.io/tls-acme: "true"
    nginx.ingress.kubernetes.io/backend-protocol: "HTTPS"
    nginx.ingress.kubernetes.io/configuration-snippet: "more_set_headers "Cache-Control: no-cache, no-store, max-age=0, must-revalidate";"
    nginx.ingress.kubernetes.io/rewrite-target: "/index.html"
    nginx.ingress.kubernetes.io/upstream-vhost: "app.monitormountain.com"
  name: "app-monitormountain"
  namespace: "ingress-nginx"
spec: 
  rules: 
  - host: "app.monitormountain.com"
    http: 
      paths: 
      -
        backend: 
          serviceName: "google-storage-buckets"
          servicePort: "443"
        path: "/"
  tls: 
  -
    hosts: 
    - "app.monitormountain.com"
    secretName: "app-monitormountain"

@bretep
Copy link

bretep commented Dec 31, 2019

Example cloud storage with PUBLIC_URL and multiple entries.

Within the version folder contains the service workers and all the static assets.

DE7F07EC-B809-4980-A921-614E461D4396
D20BE9DF-F6C3-4F94-8AD2-967977932D35

@unrevised6419
Copy link

unrevised6419 commented Dec 31, 2019

@bretep I think we misunderstood each other. In a CRA build is just a bunch of assets. index.html and login.html are still available, and in production you configure them as you want, as you showed above, but in development mode having that *.html at the end is not what we want.
Also PUBLIC_URL is used just to locate the root of HTML files.

@bretep
Copy link

bretep commented Dec 31, 2019

okay, I think what you are asking for is a new base path for the entry point in dev, which is why you references the PUBLIC_URL as an example. Do I understand that correctly?

@bretep
Copy link

bretep commented Dec 31, 2019

That complicates things. What about listing on multiple ports. Each port is a new entry?

@unrevised6419
Copy link

unrevised6419 commented Dec 31, 2019

which is why you references the PUBLIC_URL as an example

Yes PUBLIC_URL or homepage from package.json is used as a pathname

What about listing on multiple ports

Hmm, I think webpack-dev-server does not support multiple ports.

I propose multi entry to be something similar with next.js pages

@bretep
Copy link

bretep commented Dec 31, 2019

I’m not sure how next.js does multiple entry.

If we keep the “you can only work on one entry at a time” rule then you need to define ENTRY_HTML=index_other.html that will instruct the dev server to make that the index file.

This has the least amount of behavioral changes and allows multiple entry

If you want to work on multiple at the same time, you need to develop behind a proxy (like I do).

@unrevised6419
Copy link

rule then you need to define ENTRY_HTML=index_other.html

That was supposed in the beggining, but the idea is to have this

you need to develop behind a proxy (like I do).

I don't really understand what is your use case with the proxy. I'm using proxy just to proxy dev environment API.

@bretep
Copy link

bretep commented Jan 1, 2020

rule then you need to define ENTRY_HTML=index_other.html

That was supposed in the beggining, but the idea is to have this

I think if we focus just on making ENTRY_HTML=index_other.html work, this would meet the basic requirements for developing with multiple entries.

If I read the issue you linked correctly, there were some mixed issues there. Some can be solved with code splitting without the need of multiple entries.

you need to develop behind a proxy (like I do).

I don't really understand what is your use case with the proxy. I'm using proxy just to proxy dev environment API.

You can create routes or domains that point to separate entry HTML files was my point. Not really all that important for this conversation.

@danvc
Copy link
Author

danvc commented Jan 1, 2020

Hey guys, happy new year.

Thank you so much by the whole discussion. I’m sure that we’re going to reach some goal here.

My intention with this implementation is to allow the user add multiple entry points assuming that it’s going to generate separate bundles for each entry point.

Why?
Because security. Let’s me introduce a scenario for you to understand my point. There is an application that is defined to work with many security best practices. One for example is to not allow the user have access to the application and all content related to it (JavaScript files, images, html, etc) without being authorized through login. The user may have access only after logged in.
It’s to avoid the user have access to (included) JavaScript file/app and try to get some way to perform something the the user shouldn’t (understanding the code and injecting something, for example)

With this approach implemented, we can serve different apps in order to not have the whole application in a single file. It also avoids the user to have 2 separate (cra) applications for this purpose. This also avoid the user to eject cra and lose all benefits of it, including th futures one.

So, I hope we can reach a deal, but, independent our deal, it also needs the approvals from the reviewers, which maybe are offline for now. But I really hope to get a feedback from them.

Stay blessed

@danvc danvc changed the title All add multiple entry points 1084 Allow add multiple entry points 1084 Jan 8, 2020
@danvc
Copy link
Author

danvc commented Jan 14, 2020

Hello @iansu @amyrlam @mrmckeb @ianschmitz @petetnt , I hope you're doing great!

Please, let me know if there's something that I could be doing in order to get this PR reviewed by some of you.

Thanks in advance

@han814992037
Copy link

My project already run eject, how do I modify to support it?
thanks,
Xu

@danvc
Copy link
Author

danvc commented Jan 14, 2020

My project already run eject, how do I modify to support it?
thanks,
Xu

You could have a look on this example:
https://github.com/DanZeuss/create-react-app-multiple-entry-points

Or you could comparing the files of this PR and your files and change it in the same way.
https://github.com/facebook/create-react-app/pull/8249/files

@han814992037
Copy link

My project already run eject, how do I modify to support it?
thanks,
Xu

You could have a look on this example:
https://github.com/DanZeuss/create-react-app-multiple-entry-points

Or you could comparing the files of this PR and your files and change it in the same way.
https://github.com/facebook/create-react-app/pull/8249/files

got it,
thanks very much!

@danvc
Copy link
Author

danvc commented Feb 25, 2020

Hey guys, any news for this PR?

@unrevised6419
Copy link

I think this is still not a robust solution, it needs more testing, and there are more edge cases that need to be caught. 🙁 🤷‍♂️

Try to make a rebase on top of master. PUBLIC_URL was added in development mode.

@AliFlux
Copy link

AliFlux commented Apr 27, 2020

Hi, any update on this feature? The security issue described by @DanZeuss is quite legit.

@amyrlam amyrlam removed their request for review August 24, 2020 05:38
@enspdf
Copy link

enspdf commented Sep 23, 2020

Hey guys.. any updates for this PR?

@Etheryte
Copy link

Etheryte commented Oct 18, 2020

Stumbling upon this PR from Google while researching a similar solution, I think this suggestion has a lot of merit and could use some love to get the final push done.

One suggestion, since even the comments on appPages refer to them as entry points, as do the error messages etc, it would imo be better to call them just that, entry points. That way it's highly googlable, consistent with the terminology used elsewhere etc.

@jonmifsud
Copy link

Would like to add a tiny bit to this; we're creating a tool-kit of embeddable third-party widgets in addition to our core products. Allowing us to build with separate bundles is critical.

To be fair we don't even need the html files in this scenarios just the resulting bundles. Not too keen to eject and this seems very elegant. We've tried the approach proposed and I think it works just fine.

@OnkelTem
Copy link

OnkelTem commented Feb 5, 2022

So now you see clearly that create-react-app was not the proper tool for you if it came to splitting your htmls.

That's because CRA is actually intended for prototyping only, for quickly testing some ideas when you just need to bootstrap some simple tooling right away. The problem is that they announce it as a real tool instead! Which makes lots of confusions and frustration.

Don't use it for real projects or you'll going to regret. (like we did)

In our case, we've got a request from our customer to provide also a page with OpenAPI documentation. It could have be done quickly using this code:

import React from 'react';
import SwaggerUI from 'swagger-ui-react';
import 'swagger-ui-react/swagger-ui.css';

export default function Api() {
  return (
    <SwaggerUI url="/path/to/api.json" />
  );
}

but the output bundle size get increased by 223% or 2.5 MB (!) of some code required by just one auxiliary page.

image

I believe you'd agree that it's not acceptable.

So what options do we have? Within the CRA philosophy - just none. Apparently, ejection is not an option as it's a thing quite opposite to CRA by its nature and to why it was chosen in the first place. Because instead of a simple setup you immediately get a pile of confusing contraptions built on top of webpack and which you need to learn now.

The only option is to build a static HTML page from now on and maintain it manually.

That's all very disappointing.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.