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

Add optional tmpdirBase attribute #60

Closed
wants to merge 5 commits into from
Closed

Add optional tmpdirBase attribute #60

wants to merge 5 commits into from

Conversation

diligiant
Copy link

@diligiant diligiant commented May 29, 2017

Facebook createreactapp ModuleScopePlugin enforces all files to be children of the source directory.

A simple

tmpdirBase: paths.appSrc,

addition to the plugin configuration will ensure conformance to this rule.

create-react-app issue #2404 needs also to be fixed (PR opened) as it prevents dot directories to pass ModuleScopePlugin gatekeeper.

Facebook createreactapp ModuleScopePlugin enforced all files to be children of the source directory.

A simple `tmpdirBase: paths.appSrc,` addition to the plugin configuration will ensure conformance to this rule.
util.js Outdated
tmpdir: () => {
const tmpdir = path.resolve("./.tmp-globalize-webpack");
tmpdir: (base) => {
const tmpdir = path.resolve(base + "/.tmp-globalize-webpack");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use path.join instead of +.

@rxaviers
Copy link
Owner

Please, can you update documentation (i.e., README.md) with this additional attribute.

@rxaviers
Copy link
Owner

rxaviers commented May 29, 2017

Can you explain the motivation please, it's not immediately clear to me why these temporary files need to be inside your source path.

@diligiant
Copy link
Author

Where should I explain the motivation? Without this change, create-react-app ModuleScopePlugin raises

Module not found: You attempted to import /path_root_here/website/.tmp-globalize-webpack/dev-i18n-data.js which falls outside of the project src/ directory. Relative imports outside of src/ are not supported. You can either move it inside src/, or add a symlink to it from project's node_modules/.

As you know i18n-data.js and other files are created by the plugin (dev mode) and the globalize-compiler respectively (production mode).

@rxaviers
Copy link
Owner

Ok @diligiant thanks for the clarification. I think the above comment is enough for my understanding. For the documentation, I think it's sufficient to simply include tmpdirBase and explain what it is and mention the default is .tmp.

It's also important to add tests to ensure the different values provided to tmpdirBase work as expected.

Thanks so far.

@diligiant
Copy link
Author

Will add tests later today

@rxaviers
Copy link
Owner

Excellent, ok.

Frédéric Miserey added 2 commits May 29, 2017 20:03
@diligiant
Copy link
Author

diligiant commented May 29, 2017

@rxaviers hope the added test suits you. You'll see that I tried to be creative with the indentation of the GlobalizePlugin object ;)
And I signed-off the last commit.

my create-react-app PR has been merged so this PR might be useful after all!

@rxaviers rxaviers closed this in 1e508fe Jun 16, 2017
rxaviers pushed a commit that referenced this pull request Jun 16, 2017
Facebook createreactapp ModuleScopePlugin enforced all files to be
children of the source directory.

A simple `tmpdirBase: paths.appSrc,` addition to the plugin
configuration will ensure conformance to this rule.

Signed-off-by: Frédéric Miserey <frederic@none.net>

Closes #60
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

Successfully merging this pull request may close these issues.

2 participants