search

Polymer / pwa-starter-kit

Starter templates for building full-featured Progressive Web Apps from web components.
https://pwa-starter-kit.polymer-project.org
2.36k stars 431 forks source link

[do not merge] Update redux; remove window.process #249

Closed keanulee closed 5 years ago

keanulee commented 5 years ago

Can finally remove the window.process = hack.

Do not merge until polymer build handles .mjs files.

abdonrd commented 5 years ago

@TimvdLippe how do you build the project if the Polymer tools doesn't support .mjs?

TimvdLippe commented 5 years ago

Not yet. That is still pending.

On Mon, 15 Oct 2018, 21:35 Abdón Rodríguez Davila, notifications@github.com wrote:

@TimvdLippe https://github.com/TimvdLippe how do you build the project if the Polymer tools doesn't support .mjs?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Polymer/pwa-starter-kit/pull/249#issuecomment-430003458, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrDb6KDzC_LrgAeR-VCI1KV5VBWoioVks5ulPGMgaJpZM4Xc0nz .

keanulee commented 5 years ago

Thinking more about this, I'm really not a fan of specifying the path of an import. Consider this change:

import {
  createStore,
  compose,
  applyMiddleware,
  combineReducers   
} from 'redux/es/redux.mjs'; // previously: from 'redux'

The package.json for redux does not contain the field that points to this "browser-compatible module" build ("module": "es/redux.js" uses process.env and isn't browser-compatible), so we are forced to write out the path because our tools wouldn't otherwise know where to find this build. This forces the library vendor to preserve the file path of this build across non-breaking releases. Moreover, if using TypeScript, the definitions for these functions would no longer work (since typings rely on pkg.typings).

Separately, it's weird that the "browser-compatible module" build uses the .mjs extension since this file extension was originally created so that server Node.js code could distinguish modules (the browser already knows it's a module through the <script type=module> attribute).

It's worth thinking about whether we should introduce a new package.json field for the main "browser-compatible module" build. Alternatively, we should consider removing non-browser things (like process.env) from the main pkg.module build.