Skip to content

Conversation

@wincent
Copy link
Contributor

@wincent wincent commented Oct 14, 2020

Fixes the issues discovered in liferay-frontend/liferay-portal#429

See individual commit messages for explanations.

We had inconsistent coverage here, which meant that our recent
`@liferay/npm-bundler` release was missing some files. We need
"prepublishOnly" to ensure "build" runs before "publish", because we use
"copyfiles" to copy things like these templates from "src" to "lib":

    src/adapt/templates
    ├── export-module.ejs
    ├── index.js.ejs
    └── manifest-module.ejs

This was manifesting as described here:

    liferay-frontend/liferay-portal#429 (comment)

with:

    Error: ENOENT: no such file or directory, open
    'modules/node_modules/@liferay/npm-bundler/lib/adapt/templates/export-module.ejs'

Before this commit we had a variety of set-ups on the js-toolkit
packages:

- generator-js: had "prepublish", which did a build during install as
  well (not what we want, because it generates noise on the `yarn
  install`; everything that gets installed from the npm registry should
  already be complete and ready to go).
- js-toolkit-core: no "prepublish" or "prepublishOnly", which runs the
  risk of making a build without all the necessary files.
- js-toolkit-scripts: "prepublish" instead of "prepublishOnly".
- npm-bridge-generator: same.
- npm-bundler: like js-toolkit-core, no "prepublish" or
  "prepublishOnly".
We were producing bad paths like:

    .//Users/greghurrell/code/portal/liferay-portal/modules/apps/frontend-js/frontend-js-recharts/build/node/bundler/bundler/generated/recharts.js

leading to errors:

    ❌ EntryModuleNotFoundError: Entry module not found: Error: Can't resolve '...'

as described here:

    liferay-frontend/liferay-portal#429 (comment)

We shouldn't be prepending `./` here, because the `generatedFile` is a
`FilePath` object and it appears that it is always absolute.
As described here:

-   liferay-frontend/liferay-portal#429 (comment)
-   liferay/liferay-js-toolkit@a931aed#r43235455

our configs use a workdir of `build/node/bundler`, which means that
appending another `bundler` produces undesired nesting.
@wincent
Copy link
Contributor Author

wincent commented Oct 14, 2020

I got a preapproved credit line to merge this (via @izaera from Slack, #amirite).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants