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

Proposal: Additional JS pre-processing on module build #5161

Open
dmythro opened this issue Sep 28, 2018 · 9 comments
Open

Proposal: Additional JS pre-processing on module build #5161

dmythro opened this issue Sep 28, 2018 · 9 comments

Comments

@dmythro
Copy link

dmythro commented Sep 28, 2018

As started in #5103, would like to propose a bit more of module processing than v2 is intended to have.

My first impression on module build was "finally, the same build for all the code!", but as I see it is not actually true, @gaearon explained that modules should be in a valid JavaScript format already.

Our use case: several projects depend on "core" package which was decided to use as a node module via git (not a git submodule for couple of reasons on convenience). Now have to build "core" project files with a separate script and Babel setup. It has some components, helpers, etc. And we want to have ability to use flat imports mainly. Components use JSX, which is not supported by module build as it figured out.

Proposal: Additional JS pre-processing on module build

Let's have couple options I see for the consideration:

  1. Ability to "turn on" same preprocessing for modules as current project already has.
  2. Ability to add something to Babel "plugins" section (like "transform-react-jsx").
  3. Ability to use those modules as is in application tests (see "react-scripts test" fails for valid JS files from modules which are successfully built #5164 with example repos).

Those options would probably have a better use with ability to specify this for the "list of modules/mask", or "per module".

@gaearon
Copy link
Contributor

gaearon commented Sep 28, 2018

Things that need to be worked out:

  • How do packages opt into being "raw"? Is the list specified in the application package? Or in packages themselves?
  • What about transitive dependencies? Where do we list them? In the application, or in the packages that import them? Are dependencies of "raw" dependency packages implicitly "raw" too?
  • Should a dependency be able to introduce a "raw" transitive dependency in a minor version? If the allowed list is inside the app, this wouldn't be possible unless it's implicitly transitive.
  • Upgrading the app can break the dependencies because the tooling has changed. How to mitigate this? How to make it an informed choice? What if the same "raw" dependency is used from apps with incompatible compiler versions? How does a "raw" dependency specify which "hosts" it's compatible with?
  • How does this affect ejecting? How to avoid complicating the configs for this?
  • How does this affect testing and linting? Do we automatically run tests or lint for other "raw" packages too? Only for the app? How would we set up a "raw" package test suite in this case? If it uses different tooling, there's a risk of different behavior.
  • If we later add support for monorepos, would we assume that every package in a monorepo is "raw" by default? If yes, how to opt out? If no, do all apps in a monorepo have to specify the same list of "raw" dependencies (potentially with transitive ones)? Does it mean that adding a new "raw" dependency (or a transitive one) takes a lot of effort? Should the "raw" list be centralized? Should it be config or code?
  • Is it allowed to publish a "raw" dependency on public npm? That's dangerous for the ecosystem. But perhaps we want to allow publishing them to a private registry? How to enforce this, and is it a useful thing to enforce? What about dependencies that are neither on npm nor in a monorepo?
  • How should watching mode work? Do we automatically rebuild all "raw" dependencies? Do we run the watcher in other folders if they're linked? Does the watcher lint them with parent project settings? What happens when a "raw" dependency wants to get its own build step?
  • What do other tools do? How do we converge on common behavior with them? Does it matter?

@dmythro
Copy link
Author

dmythro commented Sep 28, 2018

Using "raw" could be for "all" or "specific", in both cases responsibility relies on application. As out-of-the-box it is "off", applying this kind of behavior should be aware.

Let's check the main goal here: for me it is ability to build packages with same kind of sources the same way.

Transitive dependencies by default is probably "as is"; also could be included if required. New dependencies in this case are also on application. Usually if I use a library, like material-ui, I check it's sources so can see it's ready for my build system (maybe with some extra sugar), but don't really dig into it's dependencies if they are commonly used.

Not sure how ejecting works right now, but I suppose this could just extend configuration for modules from the config. A good question anyway.

I'd not extend testing wider that app itself, usually packages have own tests which are used before releases, in own CI pipelines etc. Same for watch mode: usually you stop it before upgrade.

Can't say about monorepo, didn't work with it yet in a context.

About public/private: I'd not care, to be honest. Plus, there already are repos with 2 kinds of export including "next", just sources etc, and you choose what suits better.

Module can export ES5, but you will be able to use their sources with flat imports to make build actually better than that if you build for modern browsers only.

@jlongster
Copy link

I'm a monorepo user using a slight fork of react-scripts to make it sure. Here are my quick answers:

Things that need to be worked out:

  • How do packages opt into being "raw"? Is the list specified in the application package? Or in packages themselves?

It could support the config format for the tools that manage monorepos, like lerna and yarn. For yarn these are all already listed in workspaces.packages in package.json. Look for a package.json in a parent directory, and try to read the package list from there.

  • What about transitive dependencies? Where do we list them? In the application, or in the packages that import them? Are dependencies of "raw" dependency packages implicitly "raw" too?

I wouldn't think so. I'd treat only files underneath the explicit packages as "raw".

For what it's worth, in my react-scripts fork I removed the line which only includes files underneath src for babel, include: paths.appSrc: https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/webpack.config.prod.js#L276. That change alone was good enough when using yarn workspaces.

Curiously it works with exclude: /node_modules/. Yarn adds all your packages as symlinks inside the root node_modules folder, which is why everything can find each other. I guess webpack resolves those symlinks or something and the final path it tests in excludes is the resolve version (which is outside of node_modules).

Not sure what the right solution here is though. I understand wanting the include restriction.

  • Should a dependency be able to introduce a "raw" transitive dependency in a minor version? If the allowed list is inside the app, this wouldn't be possible unless it's implicitly transitive.

If we're thinking just of monorepos, I don't think this is an issue. If you're trying to find a slightly more general solution though, these questions are harder. For monorepos I think only ever sticking to the packages list would work.

  • Upgrading the app can break the dependencies because the tooling has changed. How to mitigate this? How to make it an informed choice? What if the same "raw" dependency is used from apps with incompatible compiler versions? How does a "raw" dependency specify which "hosts" it's compatible with?

If you stick with monorepos + an explicit package list this isn't a problem. You're only ever dealing with packages that you own so it's up to you to upgrade.

  • How does this affect ejecting? How to avoid complicating the configs for this?

Not sure

  • How does this affect testing and linting? Do we automatically run tests or lint for other "raw" packages too? Only for the app? How would we set up a "raw" package test suite in this case? If it uses different tooling, there's a risk of different behavior.

I don't have enough context to answer some these other questions, but it you only support monorepos & local packages that you own with this these answers seem to have easy answers (yes, support running tests for those packages)

  • If we later add support for monorepos, would we assume that every package in a monorepo is "raw" by default? If yes, how to opt out? If no, do all apps in a monorepo have to specify the same list of "raw" dependencies (potentially with transitive ones)? Does it mean that adding a new "raw" dependency (or a transitive one) takes a lot of effort? Should the "raw" list be centralized? Should it be config or code?

Yes, assume it's raw by default. Why would you want to opt out? I don't think that necessarily has to be a feature.

  • Is it allowed to publish a "raw" dependency on public npm? That's dangerous for the ecosystem. But perhaps we want to allow publishing them to a private registry? How to enforce this, and is it a useful thing to enforce? What about dependencies that are neither on npm nor in a monorepo?

Not sure about this one. I don't publish mine. I would discourage publishing raw to npm for now though.

  • How should watching mode work? Do we automatically rebuild all "raw" dependencies? Do we run the watcher in other folders if they're linked? Does the watcher lint them with parent project settings? What happens when a "raw" dependency wants to get its own build step?

Do you need to run the watcher in older folders? Right now watching "just works" for me by using the exclude: /node_modules/ instead of include. I think webpack will pick it all up correctly based on the dependency graph.

If a "raw" dependency wants its own build step, it's probably for a different environment (where you have to compile less or more features), and I'd ignore it and say that create-react-app will always import the raw version.

  • What do other tools do? How do we converge on common behavior with them? Does it matter?

This is a largely unsolved problem, so I think you should find whatever is the best for create-react-app. My only experience is with lerna which tries to run multiple processes per packages to build them separately and it gets quite messy.

@SavePointSam
Copy link

While I'm not able to answer every question explicitly. My approach, in a monorepo, is that the sibling packages have a babel.config.js file in the root that define the plugins needed to transform the source down to more generic ES5. Of course, this does get difficult in that you would have to rely on/trust published packages converting down to a generic common ancestor. Most packages do this now through their build step. There is also room for a specific entry key in the package.json of the dependency that points to the 'raw' input, while maintaining the existing tooling around the current 'main' key that points to a pre-transpiled ES5 entry.

While you can set constraints/policy for an internal dependency, things get dramatically complex when considering the wider ecosystem. Though this is a very important conversation to have as everyone in the greater node/babel community will need to orchestrate this transition together. It will certainly be a huge push and may need coordination between node/npm/babel and other leaders in the community.

@facebook facebook deleted a comment from Azure2020 Sep 28, 2018
@gaearon
Copy link
Contributor

gaearon commented Sep 28, 2018

It could support the config format for the tools that manage monorepos, like lerna and yarn. For yarn these are all already listed in workspaces.packages in package.json. Look for a package.json in a parent directory, and try to read the package list from there.

We actually tried that and it ended up being a bad decision (and removed in 2.0 stable). People can have monorepos but not want to compile stuff inside of them with host app setup because they might already have a different compile step for a subset of packages. And similarly, people sometimes want to get "raw" packages from a private npm registry rather than a monorepo, in which case putting them in monorepo config wouldn't work or make sense.

@jlongster
Copy link

People can have monorepos but not want to compile stuff inside of them with host app setup

It seems like in that case they could do import foo from "other-package/lib/file.js" which is a pre-built file. CRA might build it again but wouldn't do anything. If you want the raw file you do import foo from "other-package/src/file.js", so src and lib disambiguate whether it's raw or not.

For the case of packages from a private registry, that seems like a hard problem to solve without allowing raw packages from the public registry as well.

I am very much entrenched in my view as a user of a specific setup though. I have no idea what the right path is for CRA, but I'm always happy to put forth what my setup is for feedback.

@dmythro
Copy link
Author

dmythro commented Sep 30, 2018

Added a reference to #5164 (pre-processing for modules in tests) into description.

@thedanchez
Copy link

Just a thought and question regarding this proposal. There is a definitely a use-case where people would want to create separate ES6 modules and import them (non-transpiled) into a CRA2 module (ala yarn workspaces). What is the possibility of this proposal being worked on to address this need?

@alex-pex
Copy link

alex-pex commented Jan 3, 2019

My proposal :

both type of packages are managed using workspaces

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

No branches or pull requests

6 participants