Allow importing package.json by iamdoron · Pull Request #2468 · react/create-react-app

iamdoron · 2017-06-04T15:09:27Z

Hi,

Basically, allow to import package.json.
For example, from src/App.js:

import{version}from'../package.json'

Only the app own package.json is allowed to be imported.

It closes#2466

RohovDmytro · 2017-06-04T15:15:11Z

It it a safe practice to do so?

iamdoron · 2017-06-04T15:27:38Z

@rogovdm, I guess it depends what kind of date is in the package.json. Also, in terms of bundle size it might not be efficient to import the whole package.json.

Maybe it's better to only include the version property from the package.json

gaearon

I don't think this is correct. descriptionFileRoot is package.json even if I import ../lol.json. And it lets me do that. So this is probably the wrong thing to look at.

I assume that the correct thing to look at would be requestRelative calculated below, and if it's equal exactly to ../package or ../package.json (since we can't use Webpack resolving mechanism yet).

iamdoron · 2017-06-22T20:15:13Z

+ const requestRelativeToRoot = path.relative(
+ request.descriptionFileRoot,
+ requestFullPath
+ );


@gaearon you are totally right. My previous code wasn't working . I'm not sure what I did there, but this is what I meant to do - to check this is the package.json that's in the root. I didn't want to assume package.json always lives in ../src in case the src is more than one level deep like true/src

gaearon · 2017-06-22T20:15:43Z

I don't understand newly pushed commits. I was referring to using existing variable (which has the right thing). You renamed an old variable but it still calculates the wrong thing.

iamdoron · 2017-06-22T20:20:08Z

I think it's OK, but maybe I'm missing something
there are two types or "relatives".
relative to src
relative to root

I wanted to check this is the package.json that resides in root:

constrequestRelativeToRoot=path.relative(request.descriptionFileRoot,requestFullPath);

gaearon · 2017-06-22T20:22:34Z

I'm not sure myself. Just make sure importing ../somethingelse.json is still an error.

iamdoron · 2017-06-22T20:23:09Z

now when I run it:

require("../../package_.json")

Module not found: You attempted to import ../../package_.json

require("../package_.json")

Module not found: You attempted to import ../package.json

and require("../../package.json") works because it's inside the root

Timer · 2017-06-22T20:30:48Z

Wouldn't it be easier to simply use this variable and add:

if(requestRelative===`..${path.sep}package.json`){returncallback();}

I might be missing something obvious. I have not tested this.

iamdoron · 2017-06-22T20:34:39Z

@Timer I wanted it to check from the root and not src/../package.json in case the source folder is two levels or more from the root
but maybe it's enough to assume that src/../package.json is the right one

Timer · 2017-06-22T20:41:12Z

Good point, @iamdoron.
Maybe a regex pattern like this?

if(/^(..[/|\\])+package.json$/.test(requestRelative)){returncallback();}

iamdoron · 2017-06-22T20:59:37Z

+ request.__innerRequest_request
 );
+ const requestRelative = path.relative(appSrc, requestFullPath);
+ if (/^(..[/|\\])+package(.json)?$/.test(requestRelative)) {


@Timer I changed it to regex, I added '?' in (.json)? to make it optional. that way you can require("../package")

It should always include the extension no matter what.

viankakrisna · 2017-06-22T21:21:49Z

How about adding excludes option to ModuleScopePlugin?

 new ModuleScopePlugin({
appSrc: paths.appSrc,
excludes: [paths.appPackageJson]
}),

 if (excludes.includes(request.path)){
return callback()
}

Timer · 2017-06-22T23:31:29Z

@viankakrisna that's probably best long-term; what do you think @iamdoron?

gaearon · 2017-06-27T16:13:45Z

What's the status on this? What are requested changes?

Timer · 2017-06-28T20:47:10Z

We should strictly enforce the .json extension so people don't use this as an escape hatch to import package.js from the root.

Timer

Looks great!

Timer · 2017-06-29T17:42:34Z


 class ModuleScopePlugin {
- constructor(appSrc) {
+ constructor(appSrc, allowedPaths = []) {


Small nit, how about allowedFiles? Paths makes it sound like you can give it a directory.

Timer · 2017-06-29T17:43:15Z



-#### `new ModuleScopePlugin(appSrc: string)`
+#### `new ModuleScopePlugin(appSrc: string, allowedPaths: [string])`


Ditto, allowedFiles. And we should probably let them know it's optional allowedFiles?: string[].

Outdated

br0p0p · 2017-07-31T20:59:20Z

What's the status on this?

Timer · 2017-08-02T01:55:00Z

@br0p0p waiting for CI to pass, now. 😄 thanks for the ping!

Timer · 2017-08-02T02:51:06Z

Thanks for this, @iamdoron!

Timer · 2017-08-09T03:50:05Z

Hi there! This change is out in react-scripts@1.0.11; please give it a go! Thanks.

* Allow importing package.json * Remove package.json import from App.js template * fix importing package.json * Rename variable to reflect path is relative to root * Check for both package & package.json in ModuleScopePlugin * Use regex to check relative path to package.json * Strictly enforce package.json extension on scope plugin * Add allowedPaths to ModuleScopePlugin ctor and use it to allow app package.json * Remove package.json import from App.js template * Add package.json to react-scripts/template, show package version and name in the template * Remove import package.json from template * Remove template/package.json and its references in code * Update ModuleScopePlugin.js * Update README.md

br0p0p · 2017-08-09T15:06:57Z

@Timer @iamdoron great, thanks for this!

* commit 'bfaee410c502a95076a6bd89721c76ca08e15f7b': (39 commits) Publish Prepare for 1.0.11 release (react#2924) Update dev deps (react#2923) Update README.md Use env variable to disable source maps (react#2818) Make formatWebpackMessages return all messages (react#2834) Adjust the `checkIfOnline` check if in a corporate proxy environment (react#2884) Fix the order of arguments in spawned child proc (react#2913) Feature/webpack 3 4 (react#2875) Allow importing package.json (react#2468) Re-enable flowtype warning (react#2718) Format UglifyJs error (react#2650) Unstage yarn.lock pre-commit (react#2700) Update README.md Update README.md Add Electrode to alternatives (react#2728) Fix parsing HTML/JSX tags to real elements (react#2796) Update webpack version note (react#2798) Use modern syntax feature (react#2873) Allow use of scoped packages with a pinned version (react#2853) ... # Conflicts: # packages/react-scripts/config/webpack.config.dev.js # packages/react-scripts/config/webpack.config.prod.js # packages/react-scripts/package.json

* Allow importing package.json * Remove package.json import from App.js template * fix importing package.json * Rename variable to reflect path is relative to root * Check for both package & package.json in ModuleScopePlugin * Use regex to check relative path to package.json * Strictly enforce package.json extension on scope plugin * Add allowedPaths to ModuleScopePlugin ctor and use it to allow app package.json * Remove package.json import from App.js template * Add package.json to react-scripts/template, show package version and name in the template * Remove import package.json from template * Remove template/package.json and its references in code * Update ModuleScopePlugin.js * Update README.md

facebook-github-bot added the CLA Signed label Jun 4, 2017

iamdoron mentioned this pull request Jun 4, 2017
App Version #2466
Closed

Timer added the tag: enhancement label Jun 19, 2017

Timer added this to the 1.0.x milestone Jun 19, 2017

gaearon previously requested changes Jun 22, 2017
View reviewed changes

iamdoron added 3 commits June 22, 2017 22:54

Allow importing package.json
6727da4

Remove package.json import from App.js template
d22a9df

fix importing package.json
9ef9fcb

iamdoron force-pushed the master branch from f58f9d0 to 9ef9fcbCompare June 22, 2017 19:54

iamdoron added 2 commits June 22, 2017 23:01

Rename variable to reflect path is relative to root
d382a94

Check for both package & package.json in ModuleScopePlugin
18ee2fc

iamdoron commented Jun 22, 2017
View reviewed changes

iamdoron force-pushed the master branch 2 times, most recently from 9be52d8 to be60e23Compare June 22, 2017 20:56

Use regex to check relative path to package.json
be60e23

iamdoron commented Jun 22, 2017
View reviewed changes

Strictly enforce package.json extension on scope plugin
c288eca

Remove template/package.json and its references in code
7ac9883

Timer approved these changes Jun 29, 2017
View reviewed changes

Timer modified the milestones: 1.0.10, 1.0.x Jun 29, 2017

Timer reviewed Jun 29, 2017
View reviewed changes

Timer modified the milestones: 1.0.10, 1.0.11 Jun 29, 2017

phyrog mentioned this pull request Jul 10, 2017
Remove config.js in favor of .env ontohub/ontohub-frontend#92
Merged

Timer mentioned this pull request Jul 13, 2017
Support application version via env variable #2762
Closed

Timer added 2 commits August 1, 2017 21:39

Update ModuleScopePlugin.js
fab0f91

Update README.md
daf59f4

Timer merged commit 24b18ae into react:masterAug 2, 2017

fredrikolovsson mentioned this pull request Nov 30, 2018
Ida/implement start page dina-web-nrm/dina-collections#324
Merged

lockBot locked and limited conversation to collaborators Jan 21, 2019



		#### `new ModuleScopePlugin(appSrc: string)`
		#### `new ModuleScopePlugin(appSrc: string, allowedPaths: [string])`

Uh oh!

Uh oh!

Conversation

iamdoron commented Jun 4, 2017

Uh oh!

RohovDmytro commented Jun 4, 2017

Uh oh!

iamdoron commented Jun 4, 2017

Uh oh!

gaearon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamdoronJun 22, 2017

Choose a reason for hiding this comment

Uh oh!

gaearon commented Jun 22, 2017

Uh oh!

iamdoron commented Jun 22, 2017

Uh oh!

gaearon commented Jun 22, 2017

Uh oh!

iamdoron commented Jun 22, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Timer commented Jun 22, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamdoron commented Jun 22, 2017

Uh oh!

Timer commented Jun 22, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamdoronJun 22, 2017

Choose a reason for hiding this comment

Uh oh!

TimerJun 22, 2017

Choose a reason for hiding this comment

Uh oh!

viankakrisna commented Jun 22, 2017

Uh oh!

Timer commented Jun 22, 2017

Uh oh!

gaearon commented Jun 27, 2017

Uh oh!

Timer commented Jun 28, 2017

Uh oh!

Timer left a comment

Choose a reason for hiding this comment

Uh oh!

TimerJun 29, 2017

Choose a reason for hiding this comment

Uh oh!

TimerJun 29, 2017

Choose a reason for hiding this comment

Uh oh!

br0p0p commented Jul 31, 2017

Uh oh!

Timer commented Aug 2, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Timer commented Aug 2, 2017

Uh oh!

Timer commented Aug 9, 2017

Uh oh!

br0p0p commented Aug 9, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

gaearon left a comment •
edited
Loading

iamdoron commented Jun 22, 2017•
edited
Loading

Timer commented Jun 22, 2017•
edited
Loading

Timer commented Jun 22, 2017•
edited
Loading

Timer commented Aug 2, 2017•
edited
Loading

br0p0p commented Aug 9, 2017•
edited
Loading