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

Preparing to add to Ember's default blueprint #182

Closed
ef4 opened this issue Jan 25, 2019 · 6 comments
Closed

Preparing to add to Ember's default blueprint #182

ef4 opened this issue Jan 25, 2019 · 6 comments

Comments

@ef4
Copy link
Collaborator

ef4 commented Jan 25, 2019

There is general interest in making ember-auto-import part of the default ember app blueprint. Most people will want it, and it lets us rely on it directly in the guides, etc.

This is all fine, but being part of the default install raises the bar for our long-term support and stability, and before we do that I want to make sure we don't accidentally sneak all of webpack's API into Ember's semver guarantees. Webpack is an internal implementation detail of ember-auto-import, and I want to retain the ability to switch to a different packager in the future, or to upgrade to a future webpack major version that has incompatibilities without breaking our apps.

Here are the main things I think we need to do to get ready:

  1. Document exactly what semantics we support in the imported code. Not just "whatever webpack supports", but a more explicit description like

    • how we prioritize the various keywords in package.json (main vs browser vs modules)
    • what authoring formats (es modules, common JS, etc) are supported.
    • can we do anything to make sure no webpack-specific idioms are accidentally supported in the imported code? For example: inline loader configs.

    As part of this, I'm fine with changing our internal default webpack config to be more restrictive if that helps us draw clean boundaries.

  2. Confirm that we don't accidentally support any webpack-isms in app code. I think we don't, because we don't actually hand the app code off the webpack at all. We do our own parse of it to discover imports. But we should still check that the import source strings themselves can't sneak any inline webpack options through.

  3. Provide first-class APIs for all the common things people do today via custom webpack config. And then move the ability to set custom webpack config into a separate optional addon (named something like ember-auto-import-webpack).

    This is about making it clear that you're going off the supported happy path if you want to write a custom webpack config. We can separately version the optional addon and issue breaking majors on a schedule where we wouldn't be willing to issue breaking majors in ember-auto-import itself.

    Examples of things that probably need first-class API:
    - how can a library like moment avoid packaging all locales?
    - indirect aliasing that lets you take into account custom fields in package.json
    - controlling babel transpilation of imported code.

  4. Establish a non-monkey-patching way to integrate into ember-cli. Today we patch one method to get our content into vendor.js late enough in the game.

    One solution here is to not append anything to vendor.js at all, but rather add new script tags to index.html for each chunk. The benefit here is that it gives us greater flexibility to optimize caching. If people are OK with that solution, it would be cleaner and easier, and it aligns us better with how the upstream packagers expect to be used. It does mean that in the simple case you'd have one additional script download (app.js, vendor.js, and chunk-xxxxx.js).

    Alternatively, we may need to be given a new hook in ember-cli or ember-cli needs to refactor itself back into constructing a correct broccoli dependency graph like it did before the packager refactor.

@tomdale
Copy link

tomdale commented Jan 31, 2019

Regarding not exposing non-standard webpack features, I asked @TheLarkInn about this at TC39 and he pointed me at this list of default module types that can be overridden: https://github.com/webpack/webpack/blob/e1df721fd77634b5a22b4349c7d7ea15f0a2188e/lib/WebpackOptionsDefaulter.js#L62-L87.

By default, webpack tries to discover dependencies by analyzing files for many different module systems. We can change the configuration to javascript/esm to constrain parsing and dependency discovery to ES modules.

That said, the more I've thought about this, the more I realize that we may not have much choice about being strict here. So many packages that people try to import from npm are using CommonJS, or themselves have a transitive dependency on a CommonJS module, I'm not sure if locking this down is worth the harm to the user experience.

@ef4
Copy link
Collaborator Author

ef4 commented Jan 31, 2019

I agree that we don't want to support only ESM, that's not what I really meant. CommonJS is not a webpack-ism, it's another standard that's reasonable to include in our support.

The webpack-isms I want to eliminate are things like inline loader configs:

import something from "custom-loader!the-realpackage";

And magic comments:

import(
  /* webpackInclude: /\.json$/ */
  /* webpackExclude: /\.noimport\.json$/ */
  /* webpackChunkName: "my-chunk-name" */
  /* webpackMode: "lazy" */
  /* webpackPrefetch: true */ 
  /* webpackPreload: true */
  `./locale/${language}`
);

@ef4
Copy link
Collaborator Author

ef4 commented Jan 31, 2019

It's possible we can kill some of these via babel before handing to webpack.

@MelSumner
Copy link

@ef4 curious- how is this work going?

@ef4
Copy link
Collaborator Author

ef4 commented Sep 26, 2019

There has been progress since this was written.

  • RFC 507 covers the "Document exactly what semantics we support in the imported code " piece. This may not be explicitly obvious to readers, but ember-auto-import is intended to follow exactly the same spec as embroider. We should probably say that clearly on ember-auto-import's own docs.
  • "controlling babel transpilation of imported code" is implemented and released.

Some of the rest falls into things I will naturally be prioritizing as part of embroider, but it would be helpful if we could recruit another volunteer to champion getting ember-auto-import into the stock blueprint.

@ef4
Copy link
Collaborator Author

ef4 commented Mar 23, 2021

This got done way back in ember-cli 3.14

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